🎨 Palette: Enhance UploadResumeModal accessibility#495
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. |
![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 improves the accessibility of the UploadResumeModal by adding proper ARIA attributes, implementing keyboard-accessible focus rings for buttons, and fixing the file input visibility using peer classes. I have provided feedback regarding the missing focus trap for the modal and a correction for the disabled state styling on the file upload label.
| <div className="bg-white rounded-xl shadow-2xl max-w-2xl w-full mx-4 max-h-[90vh] overflow-hidden"> | ||
| <div | ||
| role="dialog" | ||
| aria-modal="true" |
There was a problem hiding this comment.
While adding aria-modal="true" is a significant improvement for accessibility, the general rules for this repository require all modals to implement a focus trap. This ensures that keyboard navigation remains contained within the modal while it is active, preventing focus from leaking to background elements. Consider using a library like react-focus-lock or implementing a custom focus trap to satisfy this requirement.
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-accent peer-focus-visible:ring-offset-2" |
There was a problem hiding this comment.
The disabled:opacity-50 utility does not work on elements because they do not support the :disabled pseudo-class. Since the associated file input has the peer class, you should use peer-disabled:opacity-50 to correctly style the label when the input is disabled. Additionally, adding peer-disabled:cursor-not-allowed provides better visual feedback for keyboard and mouse users.
| 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-accent peer-focus-visible:ring-offset-2" | |
| className="btn-primary inline-flex items-center gap-2 px-6 py-3 cursor-pointer peer-disabled:cursor-not-allowed peer-disabled:opacity-50 peer-focus-visible:ring-2 peer-focus-visible:ring-accent peer-focus-visible:ring-offset-2" |
|
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
role="dialog",aria-modal="true", andaria-labelledbyto the upload modal. Fixed the file input's keyboard accessibility by replacinghiddenwithsr-only peerand addingpeer-focus-visibleto its label. Addedaria-labelandfocus-visiblestyles to all modal buttons.hiddenwhich removed it from the tab order, making it impossible to upload files using only a keyboard. The close and action buttons lacked visible focus indicators, hindering keyboard navigation.PR created automatically by Jules for task 2339022617521647010 started by @aafre