🎨 Palette: Add accessible close button to UploadResumeModal#537
Conversation
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 UploadResumeModal by adding standard button attributes, descriptive ARIA labels, and explicit focus-visible styling. It also updates the project's documentation to include guidelines for accessible close buttons. Review feedback suggests further enhancing the close button's touch target size and visual alignment, as well as ensuring the modal implements a focus trap. Additionally, it is recommended to update the documentation to include best practices for decorative icons.
| onClick={handleCloseModal} | ||
| className="text-white/80 hover:text-white transition-colors" | ||
| aria-label="Close upload modal" | ||
| className="text-white/80 hover:text-white transition-colors rounded-lg focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 focus-visible:ring-offset-ink" |
There was a problem hiding this comment.
To improve the touch target size and ensure the focus ring has adequate spacing from the icon, consider adding padding (e.g., p-1). Using a negative margin (-mr-1) can help maintain the visual alignment with the header's edge. Additionally, ensure this modal implements a focus trap to keep keyboard focus within the modal's focusable elements while it is open.
| className="text-white/80 hover:text-white transition-colors rounded-lg focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 focus-visible:ring-offset-ink" | |
| className="text-white/80 hover:text-white transition-colors rounded-lg p-1 -mr-1 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 focus-visible:ring-offset-ink" |
References
- For accessibility, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open.
|
|
||
| ## 2025-02-14 - Accessible Close Buttons in Dialogs | ||
| **Learning:** Icon-only close buttons in custom modals (like `UploadResumeModal`) often miss `aria-label`, `type="button"`, and explicit focus styling, leading to an inaccessible keyboard and screen reader experience. | ||
| **Action:** Ensure all icon-only modal close buttons use `type="button"`, provide a descriptive `aria-label`, and include `focus-visible:outline-none focus-visible:ring-2` styling. |
There was a problem hiding this comment.
Ensure documentation is updated to reflect UI and implementation changes. This action should be expanded to include key accessibility best practices for icon-only buttons, such as explicitly hiding decorative icons from screen readers and ensuring a sufficient touch target size.
| **Action:** Ensure all icon-only modal close buttons use `type="button"`, provide a descriptive `aria-label`, and include `focus-visible:outline-none focus-visible:ring-2` styling. | |
| **Action:** Ensure all icon-only modal close buttons use type="button", provide a descriptive aria-label, include aria-hidden="true" on the icon, and provide a sufficient touch target (e.g., p-1) with focus-visible:outline-none focus-visible:ring-2 styling. |
References
- Ensure documentation, such as testing plans, is updated to reflect UI and implementation changes made in the same pull request.
What: Added
aria-label="Close upload modal",type="button", and keyboard focus styling (focus-visible) to theUploadResumeModalclose button.Why: Improves screen reader accessibility for icon-only buttons and ensures keyboard users can visually track focus.
Before/After: The close button previously had no accessible name or visible focus ring. Now it is clearly identifiable by assistive tech and keyboard navigation.
Accessibility: Added descriptive ARIA label, prevented accidental form submission with
type="button", and added Tailwindfocus-visibleclasses.PR created automatically by Jules for task 18364583551958395533 started by @aafre