🎨 Palette: Add focus states and ARIA labels to TabbedHelpModal close button#567
🎨 Palette: Add focus states and ARIA labels to TabbedHelpModal close button#567aafre wants to merge 1 commit into
Conversation
…button Co-authored-by: aafre <8656674+aafre@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. |
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility of the close button in the TabbedHelpModal component by adding explicit focus styles, a button type, and an ARIA label, alongside documenting these guidelines in .jules/palette.md. Feedback was provided to add padding and a hover background to the close button to increase its touch target size and improve the visual layout of the focus ring.
| type="button" | ||
| onClick={onClose} | ||
| className="text-gray-400 hover:text-gray-600 transition-colors" | ||
| className="text-gray-400 hover:text-gray-600 transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2 rounded-lg" |
There was a problem hiding this comment.
The close button currently lacks padding, resulting in a very small interactive target area (24x24 pixels) that is difficult to tap on mobile devices and violates touch target size accessibility guidelines. Additionally, without padding, the focus ring is drawn tightly against the edges of the icon, which looks cramped. Adding padding (e.g., 'p-1.5') and a subtle hover background (like 'hover:bg-gray-100') will improve both the touch target size and the visual appearance of the focus ring, aligning it with other close buttons in the codebase (such as the one in 'PreviewModal').
| className="text-gray-400 hover:text-gray-600 transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2 rounded-lg" | |
| className="p-1.5 text-gray-400 hover:text-gray-600 hover:bg-gray-100 transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2 rounded-lg" |
💡 What: Added
type="button", anaria-label="Close help modal", and explicitfocus-visiblestyles to the customMdClosebutton inside theTabbedHelpModal. Also created apalette.mdjournal entry detailing this learning.🎯 Why: Custom icon-only modal close buttons without explicit focus outlines fail accessibility standards because they do not provide clear visual feedback to keyboard users (even if they have hover states). Without an
aria-label, screen readers may not accurately describe the action. Addingtype="button"ensures it doesn't accidentally trigger forms.♿ Accessibility:
aria-label="Close help modal"for screen readers.focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2for keyboard navigators.type="button"to avoid unintended form behavior.PR created automatically by Jules for task 14399274600887456440 started by @aafre