fix: improve accessibility of @ context menu for screen readers (#3186) #5918
Conversation
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
- Add proper ARIA roles and properties to ContextMenu component - Add role="listbox" and aria-label to menu container - Add role="option" and aria-selected to menu items - Add aria-expanded, aria-haspopup, and aria-controls to textarea - Add live region for screen reader announcements - Announce menu state changes (open/close) - Announce selected menu items with position info - Add instructions for screen reader users - Improve keyboard navigation accessibility Fixes #3186
dosubot
bot
added
size:L
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| setShowContextMenu(showMenu) | ||
|
|
||
| // Announce menu state changes for screen readers | ||
| if (showMenu && !wasMenuVisible) { | ||
| setScreenReaderAnnouncement("File insertion menu opened") |
There was a problem hiding this comment.
Consider localizing the screen reader announcement strings (e.g., "File insertion menu opened" and "closed") using the translation function instead of hardcoding English messages.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| @@ -559,6 +568,48 @@ | |||
| } | |||
| }, [showContextMenu]) | |||
|
|
|||
| // Announce selected menu item for screen readers | |||
| useEffect(() => { | |||
There was a problem hiding this comment.
The useEffect that announces the selected menu item builds its messages with hardcoded strings. Consider using the translation function (t) to ensure these announcements are localized.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| ref={menuRef} | ||
| role="listbox" | ||
| aria-label="File insertion menu" |
There was a problem hiding this comment.
The ARIA label for the menu ('File insertion menu') is hardcoded. To support internationalization, consider using a translation function for this label.
| aria-label="File insertion menu" | |
| aria-label={t('File insertion menu')} |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
There was a problem hiding this comment.
Hey @MuriloFP , I left points that need to be fixed for this PR, also let me know if you can test it
| switch (selectedOption.type) { | ||
| case ContextMenuOptionType.File: | ||
| case ContextMenuOptionType.OpenedFile: | ||
| announcement = `File: ${selectedOption.value || selectedOption.label}, ${selectedMenuIndex + 1} of ${options.length}` |
There was a problem hiding this comment.
All these announcement strings need to be translated. Consider creating a helper function that uses the translation system:
const getAnnouncementText = (option: ContextMenuQueryItem, index: number, total: number) => {
const position = t("chat:contextMenu.position", { current: index + 1, total });
switch (option.type) {
case ContextMenuOptionType.File:
case ContextMenuOptionType.OpenedFile:
return t("chat:contextMenu.announceFile", {
name: option.value || option.label,
position
});
// ... other cases
}
};|
|
||
| {/* Instructions for screen readers */} | ||
| <div id="context-menu-instructions" className="sr-only"> | ||
| Type @ to open file insertion menu. Use arrow keys to navigate, Enter to select, Escape to close. |
There was a problem hiding this comment.
This instruction text should also be translated:
<div id="context-menu-instructions" className="sr-only">
{t("chat:contextMenu.instructions")}
</div>| @@ -559,6 +568,48 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>( | |||
| } | |||
| }, [showContextMenu]) | |||
|
|
|||
| // Announce selected menu item for screen readers | |||
| useEffect(() => { | |||
There was a problem hiding this comment.
This useEffect runs on every selectedMenuIndex change during keyboard navigation, which could impact performance. Consider debouncing the announcements or only announcing when navigation pauses:
useEffect(() => {
if (!showContextMenu || selectedMenuIndex < 0) return;
const timeoutId = setTimeout(() => {
// announcement logic here
}, 100); // Small delay to avoid rapid announcements
return () => clearTimeout(timeoutId);
}, [showContextMenu, selectedMenuIndex, /* other deps */]);| aria-live="polite" | ||
| aria-atomic="true" | ||
| className="sr-only" | ||
| style={{ |
There was a problem hiding this comment.
The live region implementation uses inline styles. For consistency with the rest of the codebase, consider using Tailwind utility classes:
<div
aria-live="polite"
aria-atomic="true"
className="sr-only absolute -left-[10000px] w-px h-px overflow-hidden"
>
{screenReaderAnnouncement}
</div>Note: The sr-only class might already provide the necessary styling for screen reader-only content.
|
@roomote-agent Check the reviews left on this PR and address them by using the switch_mode tool and switching to pr-fixer mode. |
|
@daniel-lxs I have reviewed the feedback on this PR. I will now switch to pr-fixer mode to address all the review comments:
I will implement these changes and push them to this PR branch. |
- Add translations for all announcement strings using helper function - Translate instruction text for screen readers - Add debouncing to useEffect to improve performance during keyboard navigation - Replace inline styles with Tailwind classes for live region - Import missing ContextMenuQueryItem type - Fix ESLint warning by adding t to dependency array
|
@roomote-agent fix the failing unit tests by switching (switch_tool) to pr-fixer mode and fix the failing tests please, commit the changes to this PR, do not create a new one. |
|
Hi @daniel-lxs! I see your request. I will switch to pr-fixer mode to address the review comments and fix the failing tests. I'll work on this PR directly without creating a new one. |
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Changes Requested]
in Roo Code Roadmap
hannesrudolph
added
PR - Changes Requested
and removed
PR - Needs Preliminary Review
labels
Related GitHub Issue
Closes: #3186
Roo Code Task Context (Optional)
No Roo Code task context for this PR
Description
This PR implements comprehensive accessibility improvements for the @ context menu to make it fully accessible to screen readers. The issue reported that when users type '@' to trigger the file insertion context menu, the menu appears visually but is not announced by screen readers, making it inaccessible to users with visual impairments.
Key implementation details:
Design choices:
Test Procedure
Manual testing with screen readers:
Keyboard navigation testing:
Pre-Submission Checklist
Screenshots / Videos
No UI changes in this PR - accessibility improvements are semantic and announced by screen readers
Documentation Updates
Additional Notes
Accessibility standards compliance:
Technical considerations:
Get in Touch
@roomote-agent
Important
Improves accessibility of the @ context menu for screen readers by adding ARIA roles, states, live regions, and enhancing keyboard navigation in
ChatTextArea.tsxandContextMenu.tsx.role="listbox",role="option") and states (aria-expanded,aria-selected,aria-activedescendant) inChatTextArea.tsxandContextMenu.tsx.ChatTextArea.tsx.ChatTextArea.tsx.chat.json.ChatTextArea.tsx.chat.jsonwith new strings for screen reader announcements and instructions.This description was created by
for 7e5c486. You can customize this summary. It will automatically update as commits are pushed.