From 2d3dd9e35105ab4fba5929a52a77930b0dbf3085 Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Wed, 22 Jan 2025 03:54:13 +0100
Subject: [PATCH] =?UTF-8?q?=E2=99=BF=EF=B8=8F=20a11y:=20Enhance=20Accessib?=
 =?UTF-8?q?ility=20in=20ToolSelectDialog,=20ThemeSelector=20and=20ChatGrou?=
 =?UTF-8?q?pItem=20(#5395)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* feat: Add keyboard shortcut for theme switching and improve accessibility announcements

* fix: Improve accessibility of ToolSelectDialog close button

* feat: Enhance accessibility in ChatGroupItem component
---
 .../Prompts/Groups/ChatGroupItem.tsx          | 38 +++++++---
 .../src/components/Tools/ToolSelectDialog.tsx |  7 +-
 client/src/components/ui/ThemeSelector.tsx    | 75 ++++++++++++++-----
 3 files changed, 89 insertions(+), 31 deletions(-)

diff --git a/client/src/components/Prompts/Groups/ChatGroupItem.tsx b/client/src/components/Prompts/Groups/ChatGroupItem.tsx
index 2ac6fbcfda..b372f8a59b 100644
--- a/client/src/components/Prompts/Groups/ChatGroupItem.tsx
+++ b/client/src/components/Prompts/Groups/ChatGroupItem.tsx
@@ -27,6 +27,7 @@ function ChatGroupItem({
   const [isPreviewDialogOpen, setPreviewDialogOpen] = useState(false);
   const [isVariableDialogOpen, setVariableDialogOpen] = useState(false);
   const onEditClick = useCustomLink<HTMLDivElement>(`/d/prompts/${group._id}`);
+
   const groupIsGlobal = useMemo(
     () => instanceProjectId != null && group.projectIds?.includes(instanceProjectId),
     [group, instanceProjectId],
@@ -34,13 +35,14 @@ function ChatGroupItem({
   const isOwner = useMemo(() => user?.id === group.author, [user, group]);
 
   const onCardClick: React.MouseEventHandler<HTMLButtonElement> = () => {
-    const text = group.productionPrompt?.prompt ?? '';
-    if (!text) {
+    const text = group.productionPrompt?.prompt;
+    if (!text?.trim()) {
       return;
     }
-    const hasVariables = detectVariables(text);
-    if (hasVariables) {
-      return setVariableDialogOpen(true);
+
+    if (detectVariables(text)) {
+      setVariableDialogOpen(true);
+      return;
     }
 
     submitPrompt(text);
@@ -59,33 +61,47 @@ function ChatGroupItem({
         }
       >
         <div className="flex flex-row items-center gap-2">
-          {groupIsGlobal === true && <EarthIcon className="icon-md text-green-400" />}
+          {groupIsGlobal === true && (
+            <EarthIcon className="icon-md text-green-400" aria-label="Global prompt group" />
+          )}
           <DropdownMenu modal={false}>
             <DropdownMenuTrigger asChild>
               <button
-                id="prompts-menu-trigger"
-                aria-label="prompts-menu-trigger"
+                id={`prompt-actions-${group._id}`}
+                aria-label={`${group.name} - Actions Menu`}
+                aria-expanded="false"
+                aria-controls={`prompt-menu-${group._id}`}
+                aria-haspopup="menu"
                 onClick={(e) => {
                   e.stopPropagation();
                 }}
+                onKeyDown={(e) => {
+                  if (e.key === 'Enter' || e.key === ' ') {
+                    e.stopPropagation();
+                  }
+                }}
                 className="z-50 inline-flex h-7 w-7 items-center justify-center rounded-md border border-border-medium bg-transparent p-0 text-sm font-medium transition-all duration-300 ease-in-out hover:border-border-heavy hover:bg-surface-secondary focus:border-border-heavy focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"
               >
-                <MenuIcon className="icon-md text-text-secondary" />
+                <MenuIcon className="icon-md text-text-secondary" aria-hidden="true" />
+                <span className="sr-only">Open actions menu for {group.name}</span>
               </button>
             </DropdownMenuTrigger>
             <DropdownMenuContent
+              id={`prompt-menu-${group._id}`}
+              aria-label={`Available actions for ${group.name}`}
               className="z-50 mt-2 w-36 rounded-lg"
               collisionPadding={2}
               align="end"
             >
               <DropdownMenuItem
+                role="menuitem"
                 onClick={(e) => {
                   e.stopPropagation();
                   setPreviewDialogOpen(true);
                 }}
                 className="w-full cursor-pointer rounded-lg text-text-secondary hover:bg-surface-hover focus:bg-surface-hover disabled:cursor-not-allowed"
               >
-                <TextSearch className="mr-2 h-4 w-4" />
+                <TextSearch className="mr-2 h-4 w-4" aria-hidden="true" />
                 <span>{localize('com_ui_preview')}</span>
               </DropdownMenuItem>
               {isOwner && (
@@ -98,7 +114,7 @@ function ChatGroupItem({
                       onEditClick(e);
                     }}
                   >
-                    <EditIcon className="mr-2 h-4 w-4" />
+                    <EditIcon className="mr-2 h-4 w-4" aria-hidden="true" />
                     <span>{localize('com_ui_edit')}</span>
                   </DropdownMenuItem>
                 </DropdownMenuGroup>
diff --git a/client/src/components/Tools/ToolSelectDialog.tsx b/client/src/components/Tools/ToolSelectDialog.tsx
index 708e4de8d3..e5c18eda47 100644
--- a/client/src/components/Tools/ToolSelectDialog.tsx
+++ b/client/src/components/Tools/ToolSelectDialog.tsx
@@ -178,10 +178,11 @@ function ToolSelectDialog({
                     setIsOpen(false);
                     setCurrentPage(1);
                   }}
-                  className="inline-block text-text-tertiary hover:text-text-secondary"
-                  tabIndex={0}
+                  className="inline-block rounded-full text-text-secondary transition-colors hover:text-text-primary"
+                  aria-label="Close dialog"
+                  type="button"
                 >
-                  <X />
+                  <X aria-hidden="true" />
                 </button>
               </div>
             </div>
diff --git a/client/src/components/ui/ThemeSelector.tsx b/client/src/components/ui/ThemeSelector.tsx
index 30339bea8b..e643f05d87 100644
--- a/client/src/components/ui/ThemeSelector.tsx
+++ b/client/src/components/ui/ThemeSelector.tsx
@@ -1,7 +1,13 @@
-import React, { useContext, useCallback, useEffect } from 'react';
+import React, { useContext, useCallback, useEffect, useState } from 'react';
 import { Sun, Moon, Monitor } from 'lucide-react';
 import { ThemeContext } from '~/hooks';
 
+declare global {
+  interface Window {
+    lastThemeChange?: number;
+  }
+}
+
 const Theme = ({ theme, onChange }: { theme: string; onChange: (value: string) => void }) => {
   const themeIcons = {
     system: <Monitor />,
@@ -9,32 +15,55 @@ const Theme = ({ theme, onChange }: { theme: string; onChange: (value: string) =
     light: <Sun />,
   };
 
+  const nextTheme = theme === 'dark' ? 'light' : 'dark';
+  const label = `Switch to ${nextTheme} theme`;
+
+  useEffect(() => {
+    const handleKeyPress = (e: KeyboardEvent) => {
+      if (e.ctrlKey && e.shiftKey && e.key.toLowerCase() === 't') {
+        e.preventDefault();
+        onChange(nextTheme);
+      }
+    };
+    window.addEventListener('keydown', handleKeyPress);
+    return () => window.removeEventListener('keydown', handleKeyPress);
+  }, [nextTheme, onChange]);
+
   return (
-    <div className="flex items-center justify-between">
-      <button
-        className="cursor-pointer"
-        onClick={() => onChange(theme === 'dark' ? 'light' : 'dark')}
-        onKeyDown={(e) => {
-          if (e.key === 'Enter' || e.key === ' ') {
-            onChange(theme === 'dark' ? 'light' : 'dark');
-          }
-        }}
-        role="switch"
-        aria-checked={theme === 'dark'}
-        tabIndex={0}
-      >
-        {themeIcons[theme]}
-      </button>
-    </div>
+    <button
+      className="flex items-center gap-2 rounded-lg p-2 transition-colors hover:bg-surface-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2"
+      aria-label={label}
+      aria-keyshortcuts="Ctrl+Shift+T"
+      onClick={(e) => {
+        e.preventDefault();
+        onChange(nextTheme);
+      }}
+      onKeyDown={(e) => {
+        if (e.key === 'Enter' || e.key === ' ') {
+          e.preventDefault();
+          onChange(nextTheme);
+        }
+      }}
+    >
+      {themeIcons[theme]}
+    </button>
   );
 };
 
 const ThemeSelector = ({ returnThemeOnly }: { returnThemeOnly?: boolean }) => {
   const { theme, setTheme } = useContext(ThemeContext);
+  const [announcement, setAnnouncement] = useState('');
 
   const changeTheme = useCallback(
     (value: string) => {
+      const now = Date.now();
+      if (typeof window.lastThemeChange === 'number' && now - window.lastThemeChange < 500) {
+        return;
+      }
+      window.lastThemeChange = now;
+
       setTheme(value);
+      setAnnouncement(value === 'dark' ? 'Dark theme enabled' : 'Light theme enabled');
     },
     [setTheme],
   );
@@ -46,6 +75,13 @@ const ThemeSelector = ({ returnThemeOnly }: { returnThemeOnly?: boolean }) => {
     }
   }, [theme, setTheme]);
 
+  useEffect(() => {
+    if (announcement) {
+      const timeout = setTimeout(() => setAnnouncement(''), 1000);
+      return () => clearTimeout(timeout);
+    }
+  }, [announcement]);
+
   if (returnThemeOnly === true) {
     return <Theme theme={theme} onChange={changeTheme} />;
   }
@@ -55,6 +91,11 @@ const ThemeSelector = ({ returnThemeOnly }: { returnThemeOnly?: boolean }) => {
       <div className="absolute bottom-0 left-0 m-4">
         <Theme theme={theme} onChange={changeTheme} />
       </div>
+      {announcement && (
+        <div aria-live="polite" className="sr-only">
+          {announcement}
+        </div>
+      )}
     </div>
   );
 };