feat: improve change editor menu keyboard navigation (#831)

app/assets/javascripts/components/NotesList.tsx

@@ -6,6 +6,10 @@ import { SNNote } from '@standardnotes/snjs';
 import { observer } from 'mobx-react-lite';
 import { FunctionComponent } from 'preact';
 import { NotesListItem } from './NotesListItem';
+import {
+  FOCUSABLE_BUT_NOT_TABBABLE,
+  NOTES_LIST_SCROLL_THRESHOLD,
+} from '@/views/constants';
 
 type Props = {
   application: WebApplication;
@@ -16,9 +20,6 @@ type Props = {
   paginate: () => void;
 };
 
-const FOCUSABLE_BUT_NOT_TABBABLE = -1;
-const NOTES_LIST_SCROLL_THRESHOLD = 200;
-
 export const NotesList: FunctionComponent<Props> = observer(
   ({
     application,

app/assets/javascripts/components/NotesOptions/ChangeEditorOption.tsx

@@ -270,7 +270,7 @@ export const ChangeEditorOption: FunctionComponent<ChangeEditorOptionProps> = ({
           ...changeEditorMenuPosition,
           position: 'fixed',
         }}
-        className="sn-dropdown flex flex-col py-1 max-h-120 min-w-68 fixed overflow-y-auto"
+        className="sn-dropdown flex flex-col py-0.5 max-h-120 min-w-68 fixed overflow-y-auto"
       >
         <PremiumModalProvider state={appState.features}>
           <EditorAccordionMenu

app/assets/javascripts/components/NotesOptions/changeEditor/EditorAccordionMenu.tsx

@@ -2,6 +2,7 @@ import { Icon } from '@/components/Icon';
 import { usePremiumModal } from '@/components/Premium';
 import { KeyboardKey } from '@/services/ioService';
 import { WebApplication } from '@/ui_models/application';
+import { FOCUSABLE_BUT_NOT_TABBABLE } from '@/views/constants';
 import { SNComponent } from '@standardnotes/snjs';
 import { Fragment, FunctionComponent } from 'preact';
 import { useCallback, useEffect, useRef, useState } from 'preact/hooks';
@@ -22,6 +23,8 @@ const getGroupId = (group: EditorMenuGroup) =>
 
 const getGroupBtnId = (groupId: string) => groupId + '-button';
 
+const isElementHidden = (element: Element) => !element.clientHeight;
+
 export const EditorAccordionMenu: FunctionComponent<
   EditorAccordionMenuProps
 > = ({
@@ -34,7 +37,6 @@ export const EditorAccordionMenu: FunctionComponent<
 }) => {
   const [activeGroupId, setActiveGroupId] = useState('');
   const menuItemRefs = useRef<(HTMLButtonElement | null)[]>([]);
-  const [focusedItemIndex, setFocusedItemIndex] = useState<number>();
   const premiumModal = usePremiumModal();
 
   const isSelectedEditor = useCallback(
@@ -64,78 +66,85 @@ export const EditorAccordionMenu: FunctionComponent<
 
   useEffect(() => {
     if (
-      typeof focusedItemIndex === 'undefined' &&
+      isOpen &&
-      activeGroupId.length &&
+      !menuItemRefs.current.some((btn) => btn === document.activeElement)
-      menuItemRefs.current.length
     ) {
-      const activeGroupIndex = menuItemRefs.current.findIndex(
+      const selectedEditor = groups
-        (item) => item?.id === getGroupBtnId(activeGroupId)
+        .map((group) => group.items)
-      );
+        .flat()
-      setFocusedItemIndex(activeGroupIndex);
+        .find((item) => isSelectedEditor(item));
-    }
-  }, [activeGroupId, focusedItemIndex]);
 
-  useEffect(() => {
+      if (selectedEditor) {
-    if (
+        const editorButton = menuItemRefs.current.find(
-      typeof focusedItemIndex === 'number' &&
+          (btn) => btn?.dataset.itemName === selectedEditor.name
-      focusedItemIndex > -1 &&
+        );
-      isOpen
+        editorButton?.focus();
-    ) {
-      const focusedItem = menuItemRefs.current[focusedItemIndex];
-      const containingGroupId = focusedItem?.closest(
-        '[data-accordion-group]'
-      )?.id;
-      if (
-        !focusedItem?.id &&
-        containingGroupId &&
-        containingGroupId !== activeGroupId
-      ) {
-        setActiveGroupId(containingGroupId);
       }
-      focusedItem?.focus();
     }
-  }, [activeGroupId, focusedItemIndex, isOpen]);
+  }, [groups, isOpen, isSelectedEditor]);
 
-  useEffect(() => {
+  const handleKeyDown = (e: KeyboardEvent) => {
-    const handleKeyDown = (e: KeyboardEvent) => {
+    if (e.key === KeyboardKey.Down || e.key === KeyboardKey.Up) {
-      switch (e.key) {
+      e.preventDefault();
-        case KeyboardKey.Up: {
+    } else {
-          if (
+      return;
-            typeof focusedItemIndex === 'number' &&
+    }
-            menuItemRefs.current.length
+
-          ) {
+    let items = menuItemRefs.current;
-            let previousItemIndex = focusedItemIndex - 1;
+
-            if (previousItemIndex < 0) {
+    if (!activeGroupId) {
-              previousItemIndex = menuItemRefs.current.length - 1;
+      items = items.filter((btn) => btn?.id);
-            }
+    }
-            setFocusedItemIndex(previousItemIndex);
+
-          }
+    const currentItemIndex =
-          e.preventDefault();
+      items.findIndex((btn) => btn === document.activeElement) ?? 0;
-          break;
+
-        }
+    if (e.key === KeyboardKey.Up) {
-        case KeyboardKey.Down: {
+      let previousItemIndex = currentItemIndex - 1;
-          if (
+      if (previousItemIndex < 0) {
-            typeof focusedItemIndex === 'number' &&
+        previousItemIndex = items.length - 1;
-            menuItemRefs.current.length
-          ) {
-            let nextItemIndex = focusedItemIndex + 1;
-            if (nextItemIndex > menuItemRefs.current.length - 1) {
-              nextItemIndex = 0;
-            }
-            setFocusedItemIndex(nextItemIndex);
-          }
-          e.preventDefault();
-          break;
-        }
       }
-    };
+      const previousItem = items[previousItemIndex];
+      if (previousItem) {
+        if (isElementHidden(previousItem)) {
+          const previousItemGroupId = previousItem.closest(
+            '[data-accordion-group]'
+          )?.id;
+          if (previousItemGroupId) {
+            setActiveGroupId(previousItemGroupId);
+          }
+          setTimeout(() => {
+            previousItem.focus();
+          }, 10);
+        }
 
-    document.addEventListener('keydown', handleKeyDown);
+        previousItem.focus();
+      }
+    }
 
-    return () => {
+    if (e.key === KeyboardKey.Down) {
-      document.removeEventListener('keydown', handleKeyDown);
+      let nextItemIndex = currentItemIndex + 1;
-    };
+      if (nextItemIndex > items.length - 1) {
-  }, [focusedItemIndex, groups]);
+        nextItemIndex = 0;
+      }
+      const nextItem = items[nextItemIndex];
+      if (nextItem) {
+        if (isElementHidden(nextItem)) {
+          const nextItemGroupId = nextItem.closest(
+            '[data-accordion-group]'
+          )?.id;
+          if (nextItemGroupId) {
+            setActiveGroupId(nextItemGroupId);
+          }
+          setTimeout(() => {
+            nextItem.focus();
+          }, 10);
+        }
+
+        nextItem?.focus();
+      }
+    }
+  };
 
   const selectEditor = (item: EditorMenuItem) => {
     if (item.component) {
@@ -160,12 +169,17 @@ export const EditorAccordionMenu: FunctionComponent<
 
         return (
           <Fragment key={groupId}>
-            <div id={groupId} data-accordion-group>
+            <div
+              id={groupId}
+              data-accordion-group
+              tabIndex={FOCUSABLE_BUT_NOT_TABBABLE}
+              onKeyDown={handleKeyDown}
+            >
               <h3 className="m-0">
                 <button
                   aria-controls={contentId}
                   aria-expanded={activeGroupId === groupId}
-                  className="sn-dropdown-item focus:bg-info-backdrop justify-between py-2.5"
+                  className="sn-dropdown-item focus:bg-info-backdrop justify-between py-3"
                   id={buttonId}
                   type="button"
                   onClick={() => {
@@ -211,6 +225,7 @@ export const EditorAccordionMenu: FunctionComponent<
                     return (
                       <button
                         role="radio"
+                        data-item-name={item.name}
                         onClick={() => {
                           selectEditor(item);
                         }}
@@ -247,7 +262,7 @@ export const EditorAccordionMenu: FunctionComponent<
                 </div>
               </div>
             </div>
-            <div className="min-h-1px my-1 bg-border hide-if-last-child"></div>
+            <div className="min-h-1px bg-border hide-if-last-child"></div>
           </Fragment>
         );
       })}

app/assets/javascripts/views/constants.ts

@@ -1,6 +1,11 @@
 export const PANEL_NAME_NOTES = 'notes';
 export const PANEL_NAME_NAVIGATION = 'navigation';
+
 export const EMAIL_REGEX =
   /^([a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)$/;
+
 export const MENU_MARGIN_FROM_APP_BORDER = 5;
 export const MAX_MENU_SIZE_MULTIPLIER = 30;
+
+export const FOCUSABLE_BUT_NOT_TABBABLE = -1;
+export const NOTES_LIST_SCROLL_THRESHOLD = 200;

app/assets/stylesheets/_sn.scss

@@ -504,6 +504,11 @@
   padding-right: 3rem;
 }
 
+.py-0\.5 {
+  padding-top: 0.125rem;
+  padding-bottom: 0.125rem;
+}
+
 .sn-component .py-2\.5 {
   padding-top: 0.625rem;
   padding-bottom: 0.625rem;