🎨 Palette: improve mobile menu keyboard accessibility#60
Conversation
Adds `focus-visible` styling to the hamburger and close menu buttons, hides internal SVGs from screen readers (`aria-hidden="true"`), and removes the backdrop from the tab sequence (`tabIndex={-1}`) in `MobileMenu.tsx` to improve keyboard navigation and accessibility.
Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com>
|
👋 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility and focus indicators of the MobileMenu component by adding focus-visible rings to buttons and aria-hidden attributes to decorative SVGs. Feedback was provided to further refine the backdrop element by using cursor-pointer to correctly indicate interactivity and adding aria-hidden="true" to prevent redundant announcements for screen reader users.
| class="absolute inset-0 bg-black/30 backdrop-blur-sm cursor-default" | ||
| aria-label="Fermer le menu" | ||
| onClick={closeDrawer} | ||
| tabIndex={-1} |
There was a problem hiding this comment.
The backdrop is intended as a 'click-away' area for mouse users and is correctly removed from the tab order (tabIndex={-1}). However, it should also be hidden from screen readers using aria-hidden="true" to avoid redundant 'Close' announcements, as an explicit close button already exists in the drawer. Additionally, cursor-default is misleading for an interactive element; cursor-pointer is the standard way to indicate that the backdrop can be clicked to close the menu.
| class="absolute inset-0 bg-black/30 backdrop-blur-sm cursor-default" | |
| aria-label="Fermer le menu" | |
| onClick={closeDrawer} | |
| tabIndex={-1} | |
| class="absolute inset-0 bg-black/30 backdrop-blur-sm cursor-pointer" | |
| aria-hidden="true" | |
| onClick={closeDrawer} | |
| tabIndex={-1} |
💡 What:
focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]to the hamburger menu button and the close menu button inMobileMenu.tsx.aria-hidden="true"to the internal<svg>icons within these buttons.tabIndex={-1}andcursor-defaultto the background backdrop button.🎯 Why:
focus-visiblestyling.aria-labels ("Ouvrir le menu" and "Fermer le menu"). Hiding the inner SVG icons prevents screen readers from redundantly announcing them.tabIndex={-1}), keyboard users are not confused by landing on an invisible button; they will correctly navigate to the explicit close button instead.📸 Before/After:
Before, the hamburger and close buttons lacked a focus indicator when tabbed to, the SVG icons could potentially cause screen reader noise, and the backdrop was focusable. Now, they show a green focus ring (
#175B37), the SVGs are hidden, and the backdrop is skipped during tabbing.♿ Accessibility:
These changes directly improve keyboard navigability and screen reader experience for the mobile menu.
PR created automatically by Jules for task 9059016012755960416 started by @bobdivx