fix(a11y): WCAG 3.2.3 — add aria-labels to navigation landmarks#39244
Open
Aitema-gmbh wants to merge 2 commits intoapache:masterfrom
Open
fix(a11y): WCAG 3.2.3 — add aria-labels to navigation landmarks#39244Aitema-gmbh wants to merge 2 commits intoapache:masterfrom
Aitema-gmbh wants to merge 2 commits intoapache:masterfrom
Conversation
…onsistent navigation
Contributor
Code Review Agent Run #587846Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
6 tasks
dosubot
bot
added
change:frontend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| return ( | ||
| <StyledHeader backgroundColor={props.backgroundColor}> | ||
| <Row className="menu" role="navigation"> | ||
| <Row className="menu" role="navigation" aria-label="Page navigation"> |
Contributor
There was a problem hiding this comment.
Suggestion: The new aria-label is hardcoded in English, so it bypasses the app's i18n system and will be announced in the wrong language for non-English users. Wrap the label with the translation helper to keep accessibility text localized like other aria-label values in the codebase. [logic error]
Severity Level: Major ⚠️
- ❌ Home page sub-navigation landmark announced in English only.
- ⚠️ Inconsistent i18n for accessibility labels across UI.
Suggested change
| <Row className="menu" role="navigation" aria-label="Page navigation"> | |
| <Row | |
| className="menu" | |
| role="navigation" | |
| aria-label={t('Page navigation')} | |
| > |
Steps of Reproduction ✅
1. Configure a non-English locale in Superset so that localized UI strings are used; the
top navigation language controls are built from `useLanguageMenuItems` in
`superset-frontend/src/features/home/RightMenu.tsx:66-120`, which pulls labels from
`useLanguageMenuItems` in `superset-frontend/src/features/home/LanguagePicker.tsx:60-27`
where the aria-label itself is localized via `aria-label={t('Languages')}` at line 18.
2. Navigate to the Welcome (home) page rendered by
`superset-frontend/src/pages/Home/index.tsx:360-79`; this page renders the "Recents",
"Dashboards", "Charts", and "Saved queries" sections whose contents include the `SubMenu`
component.
3. Expand, for example, the "Dashboards" section so that `DashboardTable` mounts; in
`superset-frontend/src/features/home/DashboardTable.tsx:188-223` the component renders
`<SubMenu ... />`, which resolves to `SubMenuComponent` in
`superset-frontend/src/features/home/SubMenu.tsx:171-213`.
4. With a screen reader, navigate by landmarks on the loaded Home page: when the
navigation row rendered at `superset-frontend/src/features/home/SubMenu.tsx:213` (`<Row
className="menu" role="navigation" aria-label="Page navigation">`) is reached, the screen
reader announces the landmark as "Page navigation" in English, even though the surrounding
labels (e.g., tab labels localized with `t('Edited')`, `t('Created')` in
`ActivityTable.tsx:143-159`) respect the current locale; this contrast, along with other
localized aria-labels like `aria-label={t('Select color scheme')}` in
`plugins/plugin-chart-word-cloud/src/plugin/controls/ColorSchemeControl/index.tsx:318` and
`aria-label={t('Languages')}` in `LanguagePicker.tsx:18`, shows that this hardcoded
aria-label bypasses the established i18n pattern.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/features/home/SubMenu.tsx
**Line:** 213:213
**Comment:**
*Logic Error: The new `aria-label` is hardcoded in English, so it bypasses the app's i18n system and will be announced in the wrong language for non-English users. Wrap the label with the translation helper to keep accessibility text localized like other `aria-label` values in the codebase.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Contributor
Code Review Agent Run #8c37b6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Implements WCAG 2.1 criterion 3.2.3 (Consistent Navigation, Level AA).
aria-label="Main navigation"to the primary header navigation (Menu.tsx)aria-label="Page navigation"to the page-level sub-navigation (SubMenu.tsx)TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
Part of a series of 16 individual WCAG 2.1 accessibility PRs. See also fix(a11y): Accessibility improvements for WCAG 2.1 Level A compliance #38342.