Fix Angular template accessibility lint debt - #685
Open
fpigeonjr wants to merge 2 commits into
Open
Conversation
Fix all 58 template-accessibility ESLint findings across the root workspace (alt-text, click-events-have-key-events, interactive-supports-focus, role-has-required-aria), then promote all 11 angular.configs.templateAccessibility rules from warn to error now that debt is at zero. - Add alt="" to the decorative comment avatar image (already named by an adjacent figcaption) - Add role="presentation"/tabindex + matching keydown/keyup handlers for click-only interactive elements across header-next, tabs, hierarchical autocomplete/listbox, page, search, experimental/tabs, autocomplete-multiselect, autocomplete, sam-sds-autocomplete, and upload - Bind aria-selected on role="option" elements to each component's real selection-state signal (highlighted, _marked, checkItemSelected(...), option.checked) - Mirror aria-owns onto aria-controls for the two role="combobox" elements that were missing it - Extend/add Vitest specs asserting the new ARIA attributes and that keyboard handlers invoke the same method as click (new page.component.spec.ts covers SamPageNextComponent, previously untested) - Bump eslint-baseline.json (root: 1619 -> 1516) to reflect the resolved debt Runtime WCAG 2.1 AA decision: lint-level accessibility is accepted as the bar for this repo given the no-Storybook raw-source constraint; a follow-up issue for a test-app-harness-based axe runtime check should be filed separately (not covered by this PR).
There was a problem hiding this comment.
🟡 Changes recommended
Several keyboard paths remain incomplete or inconsistent, and test-app accessibility rules are still configured as warnings.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Resolves Angular template-accessibility lint debt through ARIA markup, keyboard support, tests, and stricter lint enforcement.
Changes:
- Adds keyboard interactions and required accessibility attributes.
- Expands component accessibility tests.
- Promotes root accessibility lint rules and lowers the warning baseline.
File summaries
| File | Description |
|---|---|
src/ui-kit/form-controls/upload/upload.template.html |
Adds keyboard-accessible upload controls. |
src/ui-kit/form-controls/upload/upload.spec.ts |
Tests upload keyboard interactions. |
src/ui-kit/form-controls/sam-sds-autocomplete/selected-result/selected-result.component.spec.ts |
Tests selected-result ARIA state. |
src/ui-kit/form-controls/sam-sds-autocomplete/selected-result/selected-result.component.html |
Adds selected option state. |
src/ui-kit/form-controls/sam-sds-autocomplete/autocomplete-search/autocomplete-search.component.spec.ts |
Tests selection ARIA and keyboard behavior. |
src/ui-kit/form-controls/sam-sds-autocomplete/autocomplete-search/autocomplete-search.component.html |
Adds controls, selection state, and keyboard handlers. |
src/ui-kit/form-controls/autocomplete/autocomplete.template.html |
Adds ARIA controls and keyboard interactions. |
src/ui-kit/form-controls/autocomplete/autocomplete.spec.ts |
Tests autocomplete accessibility behavior. |
src/ui-kit/form-controls/autocomplete-multiselect/autocomplete-multiselect.template.html |
Adds focus, roles, and selection state. |
src/ui-kit/form-controls/autocomplete-multiselect/autocomplete-multiselect.spec.ts |
Tests keyboard selection and ARIA state. |
src/ui-kit/experimental/tabs/tab-header.html |
Makes the keyboard event container focusable. |
src/ui-kit/experimental/tabs/tab-group.spec.ts |
Tests keyboard tab selection. |
src/ui-kit/experimental/tabs/tab-group.html |
Adds Enter-based tab activation. |
src/ui-kit/experimental/search/search.template.html |
Adds keyboard handling to search results. |
src/ui-kit/experimental/search/search.spec.ts |
Tests keyboard result selection. |
src/ui-kit/experimental/patterns/layout/components/page/page.template.html |
Marks the backdrop as presentational. |
src/ui-kit/experimental/patterns/layout/components/page/page.component.spec.ts |
Tests backdrop semantics and behavior. |
src/ui-kit/experimental/listbox/listbox.component.spec.ts |
Tests option ARIA state. |
src/ui-kit/experimental/listbox/listbox.component.html |
Adds focusability and option selection state. |
src/ui-kit/experimental/hierarchical/selected-result/selected-result.component.spec.ts |
Tests selected-result ARIA state. |
src/ui-kit/experimental/hierarchical/selected-result/selected-result.component.html |
Marks selected options. |
src/ui-kit/experimental/hierarchical/autocomplete/autocomplete.component.spec.ts |
Tests ARIA and keyboard interactions. |
src/ui-kit/experimental/hierarchical/autocomplete/autocomplete.component.html |
Adds focusability and keyboard selection. |
src/ui-kit/components/tabs/tabs.spec.ts |
Tests keyboard tab selection. |
src/ui-kit/components/tabs/tabs.component.ts |
Adds keyboard-accessible tab links. |
src/ui-kit/components/header-next/header.template.html |
Updates overlay and navigation semantics. |
src/ui-kit/components/header-next/header.spec.ts |
Tests overlay semantics and dismissal. |
src/ui-kit/components/comments/comment/comment.template.html |
Marks the avatar image as decorative. |
src/ui-kit/components/comments/comment/comment.spec.ts |
Tests the image text alternative. |
eslint.config.mjs |
Promotes root template-accessibility rules. |
eslint-baseline.json |
Lowers the accepted root warning count. |
Review details
Suppressed comments (2)
src/ui-kit/experimental/search/search.template.html:68
- This Enter path closes the autocomplete but never activates
RouterLink, so it does not mirror the mouse click's navigation. Dispatch the element's click or render an anchor instead.
(keydown.enter)="closeAutocomplete(result.name)"
src/ui-kit/form-controls/upload/upload.template.html:49
- This second
role="button"browse control also omits Space activation. Add the same Space handler as the first browse control and prevent default scrolling.
- Files reviewed: 31/31 changed files
- Comments generated: 11
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fix issues flagged by the Copilot code review:
- test-app/eslint.config.mjs: remove the templateAccessibility ->
warnings override so test-app enforces the same error severity as
the root workspace's eslint.config.mjs
- tabs.component.ts (SamTabsComponent): guard selectTab() against
tab.disabled for both click and the new Enter handler, and set
tabindex=-1 on disabled tab anchors so they're out of the tab order
- hierarchical/autocomplete.component.html: add Space activation
(with preventDefault) to the clear-input role="button" span,
alongside the existing Enter handler
- listbox.component.html/.ts: derive aria-selected and aria-checked
from the same canonical selection state (isChecked(), backed by
model) instead of the stale option.checked flag, which onChecked
never updated - eliminates the aria-selected/aria-checked
contradiction and keeps both in sync as options are (un)checked
- search.template.html: dispatch the result <li>'s own click on Enter
(via .currentTarget.click()) instead of calling
closeAutocomplete directly, so RouterLink's click-bound navigation
also fires on keyboard activation, matching mouse behavior
- tab-group.html (MdTabGroup): add Space activation (with
preventDefault) alongside Enter; guard both against tab.disabled,
set tabindex=-1 and aria-disabled on disabled tab labels
- autocomplete.template.html (form-controls): add Space activation
(with preventDefault) to the add-on role="button" span, alongside
the existing Enter handler
- upload.template.html: add Space activation (with preventDefault) to
both browse role="button" links and the close role="button" span;
give the close icon a contextual aria-label ("Remove <filename>")
since the font-icon class provides no accessible name
Extends/adds Vitest specs for every change above (disabled-tab
guards, Space-key activation + preventDefault, aria-selected/checked
sync across a toggle, RouterLink navigation via dispatched click,
aria-label content). All existing and new specs pass; lint baseline,
coverage floor, and format:check remain green in both workspaces.
fpigeonjr
marked this pull request as ready for review
September 3, 2026 18:04
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
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.
Description
Resolves all 58 Angular template-accessibility ESLint findings across the root workspace (
test-appalready had zero), then promotes all 11angular.configs.templateAccessibilityrules fromwarntoerrornow that debt is at zero, per #580's promotion policy.Findings fixed, by rule:
@angular-eslint/template/alt-text(1) — addedalt=""to the decorative comment avatar<img>, whose accessible name is already supplied by an adjacentusa-sr-only<figcaption>.@angular-eslint/template/click-events-have-key-events(21) +@angular-eslint/template/interactive-supports-focus(25) — addedrole="presentation"to click-to-dismiss overlays/backdrops,tabindex="-1"to containers that only bubble keydown from their focusable children, andtabindex="0"+ a matching(keydown.enter)/(keyup.enter)handler to elements with a click-only interaction (mirroring the existing convention already used elsewhere in the codebase, e.g.selected-result.component.html's close button). Touched:header-next,tabs,experimental/hierarchical/autocomplete,experimental/listbox,experimental/patterns/layout/page,experimental/search,experimental/tabs(tab-group,tab-header),form-controls/autocomplete-multiselect,form-controls/autocomplete,form-controls/sam-sds-autocomplete/autocomplete-search,form-controls/upload.@angular-eslint/template/role-has-required-aria(11) — boundaria-selectedonrole="option"elements to each component's real selection-state signal (result['highlighted'],item._marked,checkItemSelected(...),option.checked), or a literaltrue/falsewhere the list is definitionally already-selected (selected-resultcomponents) or not-yet-selected (multiselect category headers). Mirrored the existingaria-ownsvalue ontoaria-controlsfor the tworole="combobox"elements missing it.No component behavior changed — every fix is additive markup (an attribute or a keyboard handler calling the same method the click handler already called).
Runtime WCAG 2.1 AA scope (per the issue's explicit ask): lint-level accessibility is accepted as the bar for this repo, given the no-Storybook/raw-source constraint called out in #583. This PR does not add a runtime axe/Playwright check against the
test-appharness; a follow-up issue for that should be filed separately if/when the team wants it.Motivation and Context
Closes #583
Type of Change (Select One and Apply Label)
bugfixlabelenhancementlabelbreakinglabelmaintenancelabelHow to Test
npm ci && npm ci --prefix test-appnpm run lint:baseline— root warnings are now 1516 (down from 1619), 0 errors, and the a11y rule set runs aterrorseverity.npm --prefix test-app run lint:baseline— unchanged at 4/4 (test-app had no a11y debt).npm --prefix test-app test— 153 spec files / 1411 tests, all green; new/extended specs assert the ARIA attributes and that keyboard (Enter) triggers the same method as click.npm run coverage:check— floor gate passes (statements 89.31%, branches 79.31%, functions 86.73%, lines 89.29%).cd test-app && npm run build— production build succeeds.npm run format:checkandnpm run validate:publish— both clean.Expected result: all commands above exit 0; no template-accessibility warnings remain in
eslint-report.jsonfor either workspace.Screenshots (if appropriate)
N/A — no visual changes; markup/ARIA-only.
Checklist
gh-<number>-<slug>)format:checkpasses (npm run format:check)lintpasses (npm run lint)buildpasses (cd test-app && npm run build)cd test-app && npm test)