♿ fix: Improve Accessibility in Endpoints Menu/Navigation (#5123)

* fix: prevent mobile nav toggle from being focusable when not in mobile view, add types to <NavToggle/> * fix: appropriate endpoint menu item role, add up/down focus mgmt, ensure set api key is focusable and accessible * fix: localize link titles and update text color for improved accessibility in Nav component
danny-avila · Dec 28, 2024 · a423eb8 · a423eb8
1 parent d6f1ecf
commit a423eb8
Show file tree

Hide file tree

Showing 9 changed files with 137 additions and 32 deletions.
diff --git a/client/src/components/Chat/Menus/Endpoints/MenuItem.tsx b/client/src/components/Chat/Menus/Endpoints/MenuItem.tsx
@@ -38,9 +38,9 @@ const MenuItem: FC<MenuItemProps> = ({
 
   const { getExpiry } = useUserKey(endpoint);
   const localize = useLocalize();
-  const expiryTime = getExpiry();
+  const expiryTime = getExpiry() ?? '';
 
-  const onSelectEndpoint = (newEndpoint: EModelEndpoint) => {
+  const onSelectEndpoint = (newEndpoint?: EModelEndpoint) => {
     if (!newEndpoint) {
       return;
     }
@@ -95,7 +95,8 @@ const MenuItem: FC<MenuItemProps> = ({
   return (
     <>
       <div
-        role="menuitem"
+        role="option"
+        aria-selected={selected}
         className={cn(
           'group m-1.5 flex max-h-[40px] cursor-pointer gap-2 rounded px-5 py-2.5 !pr-3 text-sm !opacity-100 hover:bg-surface-hover',
           'radix-disabled:pointer-events-none radix-disabled:opacity-50',
@@ -132,8 +133,10 @@ const MenuItem: FC<MenuItemProps> = ({
             {userProvidesKey ? (
               <div className="text-token-text-primary" key={`set-key-${endpoint}`}>
                 <button
+                  tabIndex={0}
+                  aria-label={`${localize('com_endpoint_config_key')} for ${title}`}
                   className={cn(
-                    'invisible flex gap-x-1 group-hover:visible',
+                    'invisible flex gap-x-1 group-focus-within:visible group-hover:visible',
                     selected ? 'visible' : '',
                     expiryTime ? 'text-token-text-primary w-full rounded-lg p-2' : '',
                   )}
@@ -142,8 +145,20 @@ const MenuItem: FC<MenuItemProps> = ({
                     e.stopPropagation();
                     setDialogOpen(true);
                   }}
+                  onKeyDown={(e) => {
+                    if (e.key === 'Enter' || e.key === ' ') {
+                      e.preventDefault();
+                      e.stopPropagation();
+                      setDialogOpen(true);
+                    }
+                  }}
                 >
-                  <div className={cn('invisible group-hover:visible', expiryTime ? 'text-xs' : '')}>
+                  <div
+                    className={cn(
+                      'invisible group-focus-within:visible group-hover:visible',
+                      expiryTime ? 'text-xs' : '',
+                    )}
+                  >
                     {localize('com_endpoint_config_key')}
                   </div>
                   <Settings className={cn(expiryTime ? 'icon-sm' : 'icon-md stroke-1')} />

diff --git a/client/src/components/Chat/Menus/EndpointsMenu.tsx b/client/src/components/Chat/Menus/EndpointsMenu.tsx
@@ -1,7 +1,8 @@
+import { useCallback, useRef } from 'react';
 import { alternateName } from 'librechat-data-provider';
 import { Content, Portal, Root } from '@radix-ui/react-popover';
 import { useGetEndpointsQuery } from 'librechat-data-provider/react-query';
-import type { FC } from 'react';
+import type { FC, KeyboardEvent } from 'react';
 import { useChatContext, useAgentsMapContext, useAssistantsMapContext } from '~/Providers';
 import { mapEndpoints, getEntity } from '~/utils';
 import EndpointItems from './Endpoints/MenuItems';
@@ -19,6 +20,39 @@ const EndpointsMenu: FC = () => {
   const { conversation } = useChatContext();
   const { endpoint = '' } = conversation ?? {};
 
+  const menuRef = useRef<HTMLDivElement>(null);
+
+  const handleKeyDown = useCallback((event: KeyboardEvent) => {
+    const menuItems = menuRef.current?.querySelectorAll('[role="option"]');
+    if (!menuItems) {
+      return;
+    }
+    if (!menuItems.length) {
+      return;
+    }
+
+    const currentIndex = Array.from(menuItems).findIndex((item) => item === document.activeElement);
+
+    switch (event.key) {
+      case 'ArrowDown':
+        event.preventDefault();
+        if (currentIndex < menuItems.length - 1) {
+          (menuItems[currentIndex + 1] as HTMLElement).focus();
+        } else {
+          (menuItems[0] as HTMLElement).focus();
+        }
+        break;
+      case 'ArrowUp':
+        event.preventDefault();
+        if (currentIndex > 0) {
+          (menuItems[currentIndex - 1] as HTMLElement).focus();
+        } else {
+          (menuItems[menuItems.length - 1] as HTMLElement).focus();
+        }
+        break;
+    }
+  }, []);
+
   if (!endpoint) {
     console.warn('No endpoint selected');
     return null;
@@ -55,6 +89,8 @@ const EndpointsMenu: FC = () => {
             align="start"
             role="listbox"
             id="llm-endpoint-menu"
+            ref={menuRef}
+            onKeyDown={handleKeyDown}
             aria-label={localize('com_ui_endpoints_available')}
             className="mt-2 max-h-[65vh] min-w-[340px] overflow-y-auto rounded-lg border border-border-light bg-header-primary text-text-primary shadow-lg lg:max-h-[75vh]"
           >

diff --git a/client/src/components/Chat/Menus/Models/MenuButton.tsx b/client/src/components/Chat/Menus/Models/MenuButton.tsx
@@ -36,7 +36,6 @@ export default function MenuButton({
         aria-haspopup="listbox"
         aria-expanded={isExpanded}
         aria-controls="llm-menu"
-        aria-activedescendant={isExpanded ? 'selected-llm' : undefined}
         onClick={() => setIsExpanded(!isExpanded)}
       >
         {selected && selected.showIconInHeader === true && (

diff --git a/client/src/components/Chat/Menus/Models/ModelSpec.tsx b/client/src/components/Chat/Menus/Models/ModelSpec.tsx
@@ -33,10 +33,10 @@ const MenuItem: FC<MenuItemProps> = ({
   const [isDialogOpen, setDialogOpen] = useState(false);
   const { getExpiry } = useUserKey(endpoint ?? '');
   const localize = useLocalize();
-  const expiryTime = getExpiry();
+  const expiryTime = getExpiry() ?? '';
 
   const clickHandler = () => {
-    if (expiryTime == null) {
+    if (expiryTime) {
       setDialogOpen(true);
     }
     if (onClick) {
@@ -83,10 +83,12 @@ const MenuItem: FC<MenuItemProps> = ({
             {userProvidesKey ? (
               <div className="text-token-text-primary" key={`set-key-${endpoint}`}>
                 <button
+                  tabIndex={0}
+                  aria-label={`${localize('com_endpoint_config_key')} for ${title}`}
                   className={cn(
-                    'invisible flex gap-x-1 group-hover:visible',
+                    'invisible flex gap-x-1 group-focus-within:visible group-hover:visible',
                     selected ? 'visible' : '',
-                    expiryTime != null
+                    expiryTime
                       ? 'w-full rounded-lg p-2 hover:bg-gray-200 dark:hover:bg-gray-900'
                       : '',
                   )}
@@ -95,16 +97,23 @@ const MenuItem: FC<MenuItemProps> = ({
                     e.stopPropagation();
                     setDialogOpen(true);
                   }}
+                  onKeyDown={(e) => {
+                    if (e.key === 'Enter' || e.key === ' ') {
+                      e.preventDefault();
+                      e.stopPropagation();
+                      setDialogOpen(true);
+                    }
+                  }}
                 >
                   <div
                     className={cn(
-                      'invisible group-hover:visible',
-                      expiryTime != null ? 'text-xs' : '',
+                      'invisible group-focus-within:visible group-hover:visible',
+                      expiryTime ? 'text-xs' : '',
                     )}
                   >
                     {localize('com_endpoint_config_key')}
                   </div>
-                  <Settings className={cn(expiryTime != null ? 'icon-sm' : 'icon-md stroke-1')} />
+                  <Settings className={cn(expiryTime ? 'icon-sm' : 'icon-md stroke-1')} />
                 </button>
               </div>
             ) : null}

diff --git a/client/src/components/Chat/Menus/Models/ModelSpecsMenu.tsx b/client/src/components/Chat/Menus/Models/ModelSpecsMenu.tsx
@@ -1,9 +1,10 @@
-import { useMemo } from 'react';
 import { useRecoilValue } from 'recoil';
+import { useMemo, useCallback, useRef } from 'react';
 import { Content, Portal, Root } from '@radix-ui/react-popover';
 import { useGetEndpointsQuery } from 'librechat-data-provider/react-query';
 import { EModelEndpoint, isAssistantsEndpoint } from 'librechat-data-provider';
 import type { TModelSpec, TConversation, TEndpointsConfig } from 'librechat-data-provider';
+import type { KeyboardEvent } from 'react';
 import { useChatContext, useAssistantsMapContext } from '~/Providers';
 import { useDefaultConvo, useNewConvo, useLocalize } from '~/hooks';
 import { getConvoSwitchLogic, getModelSpecIconURL } from '~/utils';
@@ -88,6 +89,39 @@ export default function ModelSpecsMenu({ modelSpecs }: { modelSpecs?: TModelSpec
     return spec;
   }, [modelSpecs, conversation?.spec]);
 
+  const menuRef = useRef<HTMLDivElement>(null);
+
+  const handleKeyDown = useCallback((event: KeyboardEvent) => {
+    const menuItems = menuRef.current?.querySelectorAll('[role="option"]');
+    if (!menuItems) {
+      return;
+    }
+    if (!menuItems.length) {
+      return;
+    }
+
+    const currentIndex = Array.from(menuItems).findIndex((item) => item === document.activeElement);
+
+    switch (event.key) {
+      case 'ArrowDown':
+        event.preventDefault();
+        if (currentIndex < menuItems.length - 1) {
+          (menuItems[currentIndex + 1] as HTMLElement).focus();
+        } else {
+          (menuItems[0] as HTMLElement).focus();
+        }
+        break;
+      case 'ArrowUp':
+        event.preventDefault();
+        if (currentIndex > 0) {
+          (menuItems[currentIndex - 1] as HTMLElement).focus();
+        } else {
+          (menuItems[menuItems.length - 1] as HTMLElement).focus();
+        }
+        break;
+    }
+  }, []);
+
   return (
     <Root>
       <MenuButton
@@ -114,6 +148,8 @@ export default function ModelSpecsMenu({ modelSpecs }: { modelSpecs?: TModelSpec
               align="start"
               id="llm-menu"
               role="listbox"
+              ref={menuRef}
+              onKeyDown={handleKeyDown}
               aria-label={localize('com_ui_llms_available')}
               className="models-scrollbar mt-2 max-h-[65vh] min-w-[340px] max-w-xs overflow-y-auto rounded-lg border border-gray-100 bg-white shadow-lg dark:border-gray-700 dark:bg-gray-700 dark:text-white lg:max-h-[75vh]"
             >

diff --git a/client/src/components/Chat/Menus/UI/TitleButton.tsx b/client/src/components/Chat/Menus/UI/TitleButton.tsx
@@ -16,7 +16,6 @@ export default function TitleButton({ primaryText = '', secondaryText = '' }) {
         role="combobox"
         aria-haspopup="listbox"
         aria-controls="llm-endpoint-menu"
-        aria-activedescendant={isExpanded ? 'selected-endpoint' : undefined}
         onClick={() => setIsExpanded(!isExpanded)}
       >
         <div>

diff --git a/client/src/components/Nav/Nav.tsx b/client/src/components/Nav/Nav.tsx
@@ -213,18 +213,21 @@ const Nav = ({
         navVisible={navVisible}
         className="fixed left-0 top-1/2 z-40 hidden md:flex"
       />
-      <div
-        role="button"
-        tabIndex={0}
-        className={`nav-mask ${navVisible ? 'active' : ''}`}
-        onClick={toggleNavVisible}
-        onKeyDown={(e) => {
-          if (e.key === 'Enter' || e.key === ' ') {
-            toggleNavVisible();
-          }
-        }}
-        aria-label="Toggle navigation"
-      />
+      {isSmallScreen && (
+        <div
+          id="mobile-nav-mask-toggle"
+          role="button"
+          tabIndex={0}
+          className={`nav-mask ${navVisible ? 'active' : ''}`}
+          onClick={toggleNavVisible}
+          onKeyDown={(e) => {
+            if (e.key === 'Enter' || e.key === ' ') {
+              toggleNavVisible();
+            }
+          }}
+          aria-label="Toggle navigation"
+        />
+      )}
     </>
   );
 };

diff --git a/client/src/components/Nav/NavToggle.tsx b/client/src/components/Nav/NavToggle.tsx
@@ -10,6 +10,14 @@ export default function NavToggle({
   side = 'left',
   className = '',
   translateX = true,
+}: {
+  onToggle: () => void;
+  navVisible: boolean;
+  isHovering: boolean;
+  setIsHovering: (isHovering: boolean) => void;
+  side?: 'left' | 'right';
+  className?: string;
+  translateX?: boolean;
 }) {
   const localize = useLocalize();
   const transition = {

diff --git a/client/src/components/SidePanel/Nav.tsx b/client/src/components/SidePanel/Nav.tsx
@@ -48,10 +48,10 @@ export default function Nav({ links, isCollapsed, resize, defaultActive }: NavPr
                           }}
                         >
                           <link.icon className="h-4 w-4 text-text-secondary" />
-                          <span className="sr-only">{link.title}</span>
+                          <span className="sr-only">{localize(link.title)}</span>
                         </Button>
                       }
-                    ></TooltipAnchor>
+                    />
                   ) : (
                     <Accordion
                       key={index}
@@ -80,7 +80,7 @@ export default function Nav({ links, isCollapsed, resize, defaultActive }: NavPr
                                 <span
                                   className={cn(
                                     'ml-auto opacity-100 transition-all duration-300 ease-in-out',
-                                    variant === 'default' ? 'text-background dark:text-white' : '',
+                                    variant === 'default' ? 'text-text-primary' : '',
                                   )}
                                 >
                                   {link.label}
@@ -90,7 +90,7 @@ export default function Nav({ links, isCollapsed, resize, defaultActive }: NavPr
                           </AccordionPrimitive.Trigger>
                         </AccordionPrimitive.Header>
 
-                        <AccordionContent className="w-full dark:text-white">
+                        <AccordionContent className="w-full text-text-primary">
                           {link.Component && <link.Component />}
                         </AccordionContent>
                       </AccordionItem>