frontend: DropZoneBox: Remove role=button from label element#4647
frontend: DropZoneBox: Remove role=button from label element#4647Vaishnav-Dhaval wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Vaishnav-Dhaval The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR addresses an accessibility violation in the DropZoneBox usage by removing the incorrect role="button" behavior caused by rendering a MUI Button as a <label>, ensuring the UI uses a semantic <button> element instead.
Changes:
- Updated
UploadDialogto trigger the hidden file input via a Reactrefinstead ofcomponent="label"on the MUIButton. - Updated the
DropZoneBoxStorybook story to render a semantic<button>(nocomponent="label"). - Updated Storybook snapshots to reflect the DOM change from
<label role="button">to<button type="button">.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/components/common/Resource/UploadDialog.tsx | Replaces label-based file input triggering with a ref-driven click on a hidden <input type="file">. |
| frontend/src/components/common/DropZoneBox.stories.tsx | Removes component="label" so the story renders a proper <button>. |
| frontend/src/components/common/snapshots/DropZoneBox.UploadFiles.stories.storyshot | Snapshot update reflecting <label role="button"> → <button type="button">. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b487183 to
26dcd13
Compare
|
Thanks for that change. Can you please ask in the headlamp kubernetes slack channel for someone else to review and test this? |
26dcd13 to
d000b12
Compare
|
Nice work on the refactor in UploadDialog.tsx! The a11y fix and the useRef implementation look solid. Two minor observations for the final polish:
|
|
Thanks for the review @Champbreed @Vaishnav-Dhaval can you please reply to the review questions? |
|
hey hey. I'll mark this as draft for now to make it easier on reviewers. If you want to continue it, please mark it as ready to review when you've pushed changes. |
Summary
This PR fixes an accessibility violation in the
DropZoneBoxcomponent and its stories where a<label>element was incorrectly assigned arole="button"attribute.Related Issue
Fixes #4612
Changes
frontend/src/components/common/Resource/UploadDialog.tsxto use a Reactreffor triggering the hidden file input instead of relying on Material UI'scomponent="label"on theButton.frontend/src/components/common/DropZoneBox.stories.tsxto remove the unnecessarycomponent="label"from theUploadFilesstory.frontend/src/components/common/__snapshots__/DropZoneBox.UploadFiles.stories.storyshotto reflect the change from a<label>element to a semantic<button>element.Steps to Test
DropZoneBox:cd frontend npx vitest src/storybook.test.tsx -t DropZoneBoxUploadDialog).<button>(and not a<label>withrole="button") and that clicking it still opens the file picker.Screenshots (if applicable)
N/A (Accessibility change)
Notes for the Reviewer
component="label"to a manualrefis a standard way to handle file inputs in React while maintaining accessibility compliance (avoiding the ARIA allowed role violation).