Skip to content

[DataGrid] Make column header menu button aria-labels unique#16796

Merged
kenanyusuf merged 25 commits into
mui:masterfrom
owais635:unique-aria-labels-column-header-menu-button
Mar 11, 2025
Merged

[DataGrid] Make column header menu button aria-labels unique#16796
kenanyusuf merged 25 commits into
mui:masterfrom
owais635:unique-aria-labels-column-header-menu-button

Conversation

@owais635

@owais635 owais635 commented Mar 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fix: #16454.

@owais635 owais635 mentioned this pull request Mar 3, 2025
Closed
@owais635 owais635 changed the title update aria label [data grid] Column header menu button aria-labels should be descriptive and unique Mar 3, 2025
@zannager zannager added the scope: data grid Changes related to the data grid. label Mar 3, 2025
kenanyusuf
kenanyusuf reviewed Mar 5, 2025
View reviewed changes
Comment thread packages/x-data-grid/src/components/columnHeaders/ColumnHeaderMenuIcon.tsx Outdated
@mui-bot

mui-bot commented Mar 7, 2025 •
edited
Loading

Copy link
Copy Markdown

Localization writing tips ✍️

Seems you are updating localization 🌍 files.

Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️

  • Verify if the PR title respects the release format. Here are two examples (depending if you update or add a locale file)

    [l10n] Improve Swedish (sv-SE) locale
    [l10n] Add Danish (da-DK) locale

  • Update the documentation of supported locales by running pnpm l10n
  • Clean files with pnpm prettier.

Deploy preview: https://deploy-preview-16796--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against d38f65c

owais635 and others added 11 commits March 8, 2025 19:10
@owais635
update tests
8b6d7fc
@owais635
Merge branch 'master' into unique-aria-labels-column-header-menu-button
4fc0406
@owais635
update tests
6883ae4
@renovate @owais635
Bump @types/node to ^20.17.24 (#16874)
8a73e13 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@renovate @owais635
Bump axe-core to 4.10.3 (#16876)
fd4804b 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@renovate @owais635
Bump github/codeql-action action to v3.28.11 (#16877)
dc492c9 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@renovate @owais635
Bump pretty-quick to ^4.1.1 (#16883)
419d6d9 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@owais635
update tests
4a6f369
@owais635
Merge branch 'unique-aria-labels-column-header-menu-button' of https:…
df31e7e 
…//github.com/owais635/mui-x into unique-aria-labels-column-header-menu-button
@owais635
add test for headerName in aria-label
3f0712c
@owais635
update test name
1872b39
@owais635
owais635 requested a review from kenanyusuf March 9, 2025 13:16
@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 10, 2025
@github-actions

github-actions Bot commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 10, 2025
@kenanyusuf kenanyusuf added l10n Localization. accessibility a11y type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. labels Mar 10, 2025
@kenanyusuf

kenanyusuf commented Mar 11, 2025

Copy link
Copy Markdown
Member

@owais635 just pushed a small change that adds a new locale property specifically for the aria-label, that way we can keep the type for columnMenuLabel the same and display just "Column options" in the tooltip.

@kenanyusuf kenanyusuf changed the title [data grid] Column header menu button aria-labels should be descriptive and unique [data grid] Make column header menu button aria-labels unique Mar 11, 2025
kenanyusuf
kenanyusuf approved these changes Mar 11, 2025
View reviewed changes

@kenanyusuf kenanyusuf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for your contribution 👍

@kenanyusuf
kenanyusuf requested a review from a team March 11, 2025 09:41
@kenanyusuf kenanyusuf added needs cherry-pick The PR should be cherry-picked to master after merge. v7.x labels Mar 11, 2025
@MBilalShafi MBilalShafi changed the title [data grid] Make column header menu button aria-labels unique [DataGrid] Make column header menu button aria-labels unique Mar 11, 2025
MBilalShafi
MBilalShafi approved these changes Mar 11, 2025
View reviewed changes

@MBilalShafi MBilalShafi left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dropped a small description update suggestion, apart from that it LGTM

Comment thread packages/x-data-grid/src/constants/localeTextConstants.ts Outdated
Comment on lines +125 to +126
columnMenuLabel: 'Column options',
columnMenuAriaLabel: (columnName: string) => `${columnName} column options`,

@MBilalShafi MBilalShafi Mar 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick: Since we consistently call it column menu in the docs (example), probably this would be more relatable?
The idea is to leave the "menu" part to screen readers? I played with VoiceOver and calling it column menu made more sense.

Suggested change
columnMenuLabel: 'Column options',
columnMenuAriaLabel: (columnName: string) => `${columnName} column options`,
columnMenuLabel: 'Column menu',
columnMenuAriaLabel: (columnName: string) => `${columnName} column menu`,

@kenanyusuf kenanyusuf Mar 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since we consistently call it column menu in the docs (example)

I think this is a strong argument to refer to it as a menu rather than options. I'm going to leave the columnMenuLabel string as it is currently in master: "Menu". There is enough of a visual cue that it is a menu for columns given where the button is located.

The only change then will be adding a separate locale string for the aria-label which I will make ${columnName} column menu as you have suggested.

@kenanyusuf
kenanyusuf merged commit 71d1702 into mui:master Mar 11, 2025
@github-actions

github-actions Bot commented Mar 11, 2025

Copy link
Copy Markdown
Contributor

Cherry-pick PRs will be created targeting branches: v7.x

arminmeh added a commit to arminmeh/mui-x that referenced this pull request Mar 12, 2025
@arminmeh
manual cherry-pick from mui#16796
e768d95
@arminmeh arminmeh mentioned this pull request Mar 12, 2025
Merged
@owais635
owais635 deleted the unique-aria-labels-column-header-menu-button branch March 17, 2025 05:32
@mahirhir mahirhir mentioned this pull request Jun 29, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kenanyusuf kenanyusuf kenanyusuf approved these changes
@MBilalShafi MBilalShafi MBilalShafi approved these changes

Assignees

No one assigned

Labels

accessibility a11y l10n Localization. needs cherry-pick The PR should be cherry-picked to master after merge. scope: data grid Changes related to the data grid. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. v7.x

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[data grid] Column header menu button aria-labels should be descriptive and unique

5 participants

@owais635 @mui-bot @kenanyusuf @MBilalShafi @zannager