🎨 Palette: Improve accessibility for icon buttons#64
Conversation
Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request focuses on accessibility improvements and code formatting across the discussion composer components. It adds focus-visible ring styles, descriptive ARIA labels, and titles to interactive buttons, and hides decorative textual icons and SVGs from screen readers using aria-hidden="true". A new palette entry documenting these accessibility practices was also added. The reviewer suggests further enhancing the accessibility of the options menu in DiscussionHeader.tsx by adding appropriate ARIA roles (role="menu" and role="menuitem") and supporting keyboard navigation (closing on Escape).
| <div class="absolute right-0 top-10 z-20 mt-1 w-52 overflow-hidden rounded-xl border border-gray-200 bg-white py-1 shadow-lg"> | ||
| <button | ||
| class="block w-full px-3 py-2 text-left text-sm hover:bg-gray-50" | ||
| onClick={() => { setHeaderMenuOpen(false); if (selectedAgentId) void copyToClipboard(selectedAgentId); }} | ||
| onClick={() => { | ||
| setHeaderMenuOpen(false); | ||
| if (selectedAgentId) void copyToClipboard(selectedAgentId); | ||
| }} | ||
| disabled={!selectedAgentId} | ||
| > | ||
| Copier la clé de session |
There was a problem hiding this comment.
Since the trigger button now has aria-haspopup="true" and aria-expanded, the popup container should have role="menu" and its interactive children should have role="menuitem" to conform to the ARIA Menu Button pattern. Additionally, adding an onKeyDown handler to close the menu when the Escape key is pressed significantly improves keyboard accessibility.
<div
class="absolute right-0 top-10 z-20 mt-1 w-52 overflow-hidden rounded-xl border border-gray-200 bg-white py-1 shadow-lg"
role="menu"
onKeyDown={(e) => {
if (e.key === "Escape") {
setHeaderMenuOpen(false);
}
}}
>
<button
role="menuitem"
class="block w-full px-3 py-2 text-left text-sm hover:bg-gray-50"
onClick={() => {
setHeaderMenuOpen(false);
if (selectedAgentId) void copyToClipboard(selectedAgentId);
}}
disabled={!selectedAgentId}
>
Copier la clé de session
💡 What: Added aria-label, title, and proper focus-visible styles to icon-only buttons in ComposerInput and DiscussionHeader. Also wrapped the
>text icon in an aria-hidden span.🎯 Why: To improve screen reader experience and keyboard navigation visibility, especially preventing literal text like
>from being read aloud when a localized aria-label is present.📸 Before/After: Visual focus ring now uses the theme's green and gray colors with an offset. Screen reader will now announce 'Envoyer le message' instead of 'greater than'.
♿ Accessibility: Ensures interactive elements have descriptive names and visible focus states.
PR created automatically by Jules for task 16666060162273218563 started by @bobdivx