Skip to content

fix(a11y)-4: add accessible name to VersionDialog using aria-label#4389

Draft
Sagar-6203620715 wants to merge 1 commit intokubernetes-sigs:mainfrom
Sagar-6203620715:fix-access-4
Draft

fix(a11y)-4: add accessible name to VersionDialog using aria-label#4389
Sagar-6203620715 wants to merge 1 commit intokubernetes-sigs:mainfrom
Sagar-6203620715:fix-access-4

Conversation

@Sagar-6203620715
Copy link
Contributor

Summary

This PR fixes an accessibility issue by adding an accessible name to the VersionDialog component.

Related Issue

partially fixes #4385
Addresses point 4 from the issues #4385

Changes

  • Updated VersionDialog to include an aria-label
  • Fixed missing ARIA dialog name violation

Steps to Test

  1. Open the Version dialog in the application
  2. Run npm run frontend:test:a11y
  3. Verify no ARIA dialog name violations are reported

Screenshots (if applicable)

N/A

Notes for the Reviewer

  • This change is limited to accessibility compliance and does not affect UI behavior

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Sagar-6203620715
Once this PR has been reviewed and has the lgtm label, please assign joaquimrocha for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 20, 2026
@illume illume requested a review from Copilot January 21, 2026 13:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR attempts to fix an accessibility issue by adding an accessible name to the VersionDialog component through an aria-label attribute.

Changes:

  • Added aria-label attribute to the Dialog component in VersionDialog.tsx

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 40 to 43
aria-label={getProductName()}
open={open}
onClose={() => dispatch(uiSlice.actions.setVersionDialogOpen(false))}
title={getProductName()}
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aria-label attribute is redundant since the Dialog already has a title prop set to the same value. When using the custom Dialog component with a title prop, the DialogTitle component automatically renders an h1 element that provides the accessible name. Adding aria-label creates duplicate labeling. The proper approach would be to either:

  1. Remove the aria-label and ensure the DialogTitle has an id attribute that is referenced via aria-labelledby on the MuiDialog, or
  2. Remove the aria-label and rely on the DialogTitle's h1 element for the accessible name, or
  3. Add an id to titleProps and use aria-labelledby instead of aria-label.

Looking at other dialogs in the codebase that use MuiDialog directly (like PortForwardStartDialog), they use aria-labelledby pointing to a DialogTitle with an explicit id. However, since this uses the custom Dialog wrapper which doesn't automatically assign IDs to the title, the cleanest solution is to add titleProps with an id and use aria-labelledby.

Suggested change
aria-label={getProductName()}
open={open}
onClose={() => dispatch(uiSlice.actions.setVersionDialogOpen(false))}
title={getProductName()}
aria-labelledby="version-dialog-title"
open={open}
onClose={() => dispatch(uiSlice.actions.setVersionDialogOpen(false))}
title={getProductName()}
titleProps={{ id: 'version-dialog-title' }}

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these PRs! 🎉

Please check the development docs for how we do commit messages and how to run the frontend tests.

I would suggest trying to get one PR merged first before submitting other ones so you can learn how a project does things.

Could you please mark your other PRs as draft and try to get one merged? This will make it easier on the reviewers.

Cheers!

@illume illume marked this pull request as draft January 21, 2026 13:18
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 21, 2026
@Sagar-6203620715
Copy link
Contributor Author

Hi @illume I totally agree with you about reading the docs first before raising the PRs, I apologize to not have done it before. However, I would like to draw your attention my changes are unrelated to the CI failure
Root Cause: The failing tests in apiProxy.test.ts are not related to the accessibility changes in VersionDialog.
Issue: Tests expect error message "Unauthorized" but receive "Unreachable" instead.

on running on the main branch: npm test -- src/lib/k8s/api/v1/apiProxy.test.ts

image

If my observation is a correct should I submit a fix PR.

@Sagar-6203620715
Copy link
Contributor Author

Thanks for these PRs! 🎉

Please check the development docs for how we do commit messages and how to run the frontend tests.

I would suggest trying to get one PR merged first before submitting other ones so you can learn how a project does things.

Could you please mark your other PRs as draft and try to get one merged? This will make it easier on the reviewers.

Cheers!

@illume I will read the docs about commit message and run git commit --amend on all the PRs. I have tested the build for each PRs however some npm tests remain, will run that too and ping you once completed.

@illume illume requested a review from Copilot January 31, 2026 15:42
@illume illume marked this pull request as ready for review January 31, 2026 15:42
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 31, 2026
@k8s-ci-robot k8s-ci-robot requested a review from skoeva January 31, 2026 15:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing up the commit messages. Appreciated!

To update the snapshots you can run this.

npm run test -- -u

You can check things locally with these commands.

npm run tsc
npm run lint
npm run test
npm run format

Signed-off-by: Sagar Choudhary <sagar6203620715@gmail.com>
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 1, 2026
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@illume
Copy link
Contributor

illume commented Feb 14, 2026

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.

@illume illume marked this pull request as draft February 14, 2026 13:02
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WIP: Accessibility Issues Tracking

3 participants