🎨 Palette: Improve accessibility of ImageUploader remove button#46
🎨 Palette: Improve accessibility of ImageUploader remove button#46MacTechIN wants to merge 1 commit into
Conversation
- Added explicit aria-label to the icon-only remove button
- Added aria-hidden="true" and tabIndex={-1} to the hidden file input
- Added aria-hidden="true" to the internal icon
- Added focus-visible styles for improved keyboard navigation visibility
Co-authored-by: MacTechIN <63564804+MacTechIN@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 ImageUploader component by adding focus-visible styles, an aria-label, and hiding the close icon from screen readers, while documenting these learnings in .Jules/palette.md. However, the review feedback correctly points out that adding tabIndex={-1} and aria-hidden="true" to the hidden file input is redundant and an accessibility anti-pattern, as elements with Tailwind's hidden class are already excluded from the accessibility tree. Both the component implementation and the documented learnings should be updated to avoid this anti-pattern.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| className="hidden" | ||
| tabIndex={-1} | ||
| aria-hidden="true" | ||
| onChange={(e) => { |
There was a problem hiding this comment.
Adding aria-hidden="true" to focusable/interactive elements like <input> is an accessibility anti-pattern. Even with tabIndex={-1}, interactive elements should not be marked with aria-hidden="true" because they can still receive focus programmatically or via associated labels, which can cause screen readers to lose context or behave unpredictably.
Furthermore, since the input has className="hidden" (which applies display: none in Tailwind CSS), it is already completely removed from both the visual layout and the accessibility tree. Therefore, adding tabIndex={-1} and aria-hidden="true" is redundant and can trigger accessibility linter warnings.
className="hidden"
onChange={(e) => {
| **Learning:** Absolute positioned overlay buttons over images (like the remove image button) need explicit `focus-visible` styles with offset to prevent clipping in `overflow-hidden` containers, and hidden file inputs need `tabIndex={-1}` and `aria-hidden="true"` to be fully ignored by screen readers and keyboard navigation. | ||
| **Action:** Always add `tabIndex={-1}` and `aria-hidden="true"` to `className="hidden"` inputs, and use `focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none` on overlay buttons. |
There was a problem hiding this comment.
The documented learning and action regarding hidden inputs are incorrect:
- Elements with
display: none(applied by Tailwind'shiddenclass) are already completely omitted from both the visual layout and the accessibility tree. They cannot be focused by keyboard navigation or read by screen readers. - Adding
aria-hidden="true"to focusable/interactive elements (like<input>) is an accessibility anti-pattern. If the element ever receives focus (e.g., programmatically or via an associated label), screen readers will not announce anything, creating a focus trap/black hole.
We should update this file to avoid recommending this anti-pattern.
| **Learning:** Absolute positioned overlay buttons over images (like the remove image button) need explicit `focus-visible` styles with offset to prevent clipping in `overflow-hidden` containers, and hidden file inputs need `tabIndex={-1}` and `aria-hidden="true"` to be fully ignored by screen readers and keyboard navigation. | |
| **Action:** Always add `tabIndex={-1}` and `aria-hidden="true"` to `className="hidden"` inputs, and use `focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none` on overlay buttons. | |
| **Learning:** Absolute positioned overlay buttons over images (like the remove image button) need explicit focus-visible styles with offset to ensure proper keyboard focus visibility, and elements hidden via display: none (such as Tailwind's hidden class) are already completely ignored by screen readers and keyboard navigation without needing extra attributes. | |
| **Action:** Use focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none on overlay buttons to ensure accessibility, and avoid adding redundant or invalid accessibility attributes like aria-hidden="true" or tabIndex={-1} to hidden interactive inputs. |
💡 What
Improved the accessibility of the
ImageUploadercomponent by adding necessary ARIA attributes and focus styles.🎯 Why
Icon-only buttons require explicit
aria-labels for screen readers. Additionally, hidden inputs and decorative icons should be hidden from accessibility trees. Lastly, interactive elements need clear focus indicators for keyboard users.♿ Accessibility
aria-label={t('removeImage')}to the remove button.aria-hidden="true"to the internal<X>icon.tabIndex={-1}andaria-hidden="true"to the hidden file input.focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none focus-visible:ring-red-500to the remove button to ensure proper keyboard focus visibility, particularly over an image.PR created automatically by Jules for task 7031552208299249693 started by @MacTechIN
Note
Low Risk
UI-only accessibility attributes and Tailwind focus classes; no logic, API, or data-handling changes.
Overview
Improves keyboard and screen-reader support in
ImageUploaderwithout changing upload or clear behavior.The hidden file input gets
tabIndex={-1}andaria-hidden="true"so it stays out of the tab order and accessibility tree while the visible “select file” button still triggers it. The image remove control is an icon-only button: it now hasaria-label={t('removeImage')}, the<X>icon isaria-hidden="true", and focus-visible ring styles (ring-2,ring-offset-2, red ring) so keyboard focus stays visible on the overlay inside theoverflow-hiddenpreview.A short
.Jules/palette.mdnote captures the pattern for future hidden inputs and overlay buttons.Reviewed by Cursor Bugbot for commit b68cf0b. Bugbot is set up for automated code reviews on this repo. Configure here.