🎨 Palette: Improve keyboard accessibility for UploadResumeModal#484
🎨 Palette: Improve keyboard accessibility for UploadResumeModal#484aafre wants to merge 1 commit into
Conversation
Added aria-label and focus states to the close button and replaced 'hidden' with 'sr-only peer' on the file input to ensure it is in the keyboard tab order while staying visually hidden. 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the accessibility of the UploadResumeModal by adding focus-visible ring styles to the close button and file input label, along with an aria-label for the close button. Review feedback highlights the need for a focus trap to keep keyboard focus within the modal and suggests using peer-disabled variants for the file input label, as the standard disabled pseudo-class does not apply to label elements.
| <button | ||
| onClick={handleCloseModal} | ||
| className="text-white/80 hover:text-white transition-colors" | ||
| className="text-white/80 hover:text-white transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-white rounded-lg p-1" | ||
| aria-label="Close modal" | ||
| > | ||
| <XMarkIcon className="w-6 h-6" /> | ||
| </button> |
There was a problem hiding this comment.
While the individual accessibility improvements are valuable, this modal is missing a focus trap. According to the general rules, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open. This prevents users from accidentally tabbing into the background content.
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.
| <label | ||
| htmlFor="resume-file-input" | ||
| className="btn-primary inline-flex items-center gap-2 px-6 py-3 cursor-pointer disabled:opacity-50" | ||
| className="btn-primary inline-flex items-center gap-2 px-6 py-3 cursor-pointer disabled:opacity-50 peer-focus-visible:ring-2 peer-focus-visible:ring-offset-2 peer-focus-visible:ring-accent" |
There was a problem hiding this comment.
The disabled:opacity-50 utility does not work on a element because labels do not support the :disabled pseudo-class. Since this label is associated with a "peer" input, you should use the peer-disabled variant to correctly style the label when the file input is disabled. Additionally, the cursor should be updated to cursor-not-allowed when the input is disabled.
| className="btn-primary inline-flex items-center gap-2 px-6 py-3 cursor-pointer disabled:opacity-50 peer-focus-visible:ring-2 peer-focus-visible:ring-offset-2 peer-focus-visible:ring-accent" | |
| className="btn-primary inline-flex items-center gap-2 px-6 py-3 peer-disabled:opacity-50 peer-disabled:cursor-not-allowed cursor-pointer peer-focus-visible:ring-2 peer-focus-visible:ring-offset-2 peer-focus-visible:ring-accent" |
|
Closing duplicate — the most recent version of this change is PR #537. |
Understood. Acknowledging that this work is a duplicate and stopping work on this task. |
1 participant
💡 What
Improved the accessibility of the
UploadResumeModalcomponent by addressing two issues:aria-label="Close modal"and explicit:focus-visiblestyling to theXMarkIconbutton.className="hidden"on the<input type="file">withclassName="sr-only peer", and addedpeer-focus-visiblestyling to its adjacent<label>button.🎯 Why
aria-label, meaning screen reader users wouldn't know what the button did. It also lacked a visible focus indicator on the dark background.display: noneor Tailwind'shiddenon an input element completely removes it from the accessibility tree and keyboard tab order. Users navigating via keyboard could not focus the "Choose File" button.📸 Before/After
Before: Keyboard users could not tab to the "Choose File" button. The close button had no visual indication when focused, and screen readers read it as a blank button.
After: Keyboard users can smoothly tab to the "Choose File" button, which now displays a focus ring when focused. The close button is clearly read as "Close modal" and displays a white focus ring when tabbed to.
♿ Accessibility
PR created automatically by Jules for task 7822397832208867963 started by @aafre