Skip to content

Clear the a11y findings eslint-plugin-styled-components-a11y 2.2 adds - #3868

Merged
blarghmatey merged 3 commits into
mainfrom
a11y-styled-components-2-2
Aug 31, 2026
Merged

blarghmatey merged 3 commits into
mainfrom
a11y-styled-components-2-2

Conversation

@blarghmatey

Copy link
Copy Markdown
Member

What are the relevant tickets?

Unblocks #3851 (the dev-dependency bundle). eslint-plugin-styled-components-a11y 2.1.35 → 2.2.1 adds nine errors; landing them here keeps #3851 a pure dependency bump.

Description (What does it do?)

Two are real bugs.

  • ContractAdminPage.tsx:886 — the actions column rendered <ActionCell role="columnheader" /> with no content and no label, so the column was announced as an empty header. Now aria-label="Actions".
  • UserMenu.tsx:219UserMenuContainer is a styled.button (UserMenu.tsx:23), so the explicit role="button" was redundant. Removed.

Seven are deliberate patterns the rules cannot see through, so each gets a scoped eslint-disable-next-line with the reason written next to it rather than a blanket rule change:

Site Rule Why it stays
Card.tsx ×2 click-events-have-key-events The body click only forwards to the card's own a[data-card-link="true"] (see useClickChildLink, Card.tsx:208). Keyboard users tab straight to that anchor; a keydown handler would add a second focusable copy of one link.
VideoShortsModal.tsx:221 click-events-have-key-events, no-static-element-interactions Tap-anywhere play/pause is a pointer convenience. The same toggle is on the visible PlayPauseButton and on the slide's Enter handler.
VideoShortsModal.tsx:504 no-noninteractive-element-interactions The APG carousel slide pattern — role="group" + aria-roledescription="slide" + roving tabindex, Enter to toggle.
NavDrawer.tsx:259 no-static-element-interactions The Drawer is variant="persistent", so it is not a MUI Modal and gets no built-in Escape handling. This onKeyUp is the only keyboard way out.
format.tsx:102 no-noninteractive-tabindex The suppressed-value marker is focusable on purpose so its tooltip is reachable without a pointer. Screen readers already get the text from aria-label.

Worth a second opinion on two of these: the format.tsx marker (a <button> would satisfy the rule but there is no action behind it), and the VideoShortsModal.tsx:221 container (the duplicate-affordance argument depends on PlayPauseButton staying visible for the selected slide).

How can this be tested?

Verified against the 2.2.1 plugin on the #3851 branch, where all nine errors are gone and only the four unrelated import/no-restricted-paths errors remain. yarn workspace frontends run typecheck passes. yarn test UserMenu Card.test NavDrawer ContractAdminPage VideoShortsModal — 19 suites, 561 tests, all passing.

No reportUnusedDisableDirectives is configured, so the disable comments are inert until the plugin bump lands.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FyJ72HRTyQf6jVYmw3GZSt

Two are real: the contract admin table had an unlabelled columnheader,
and the user menu declared role="button" on a styled.button.

The other seven are deliberate patterns the rules cannot see through, so
they get a scoped disable with the reason:

- Card and the video-shorts player forward a body click to a control the
  user can already reach by keyboard (the card's own anchor, the visible
  PlayPauseButton). A keydown handler would duplicate it.
- The video-shorts slide is the APG carousel pattern: role="group" with
  a roving tabindex and Enter to toggle playback.
- NavDrawer is a persistent Drawer, so it is not a MUI Modal and gets no
  built-in Escape handling; the onKeyUp is the only keyboard exit.
- The suppressed-value marker is focusable so its tooltip is reachable
  without a pointer; there is no action to justify a button.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FyJ72HRTyQf6jVYmw3GZSt
Copilot AI balanced review requested due to automatic review settings August 31, 2026 17:12
@blarghmatey
blarghmatey requested a review from a team as a code owner August 31, 2026 17:12
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Addresses new accessibility lint findings introduced by eslint-plugin-styled-components-a11y 2.2.1.

Changes:

  • Corrects accessible semantics for action headers and the user-menu button.
  • Adds scoped lint suppressions with rationale for intentional interaction patterns.
  • Preserves keyboard-accessible tooltips, drawers, cards, and video controls.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
NavDrawer.tsx Documents and suppresses intentional Escape handling.
Card.tsx Suppresses card click-forwarding warnings.
UserMenu.tsx Removes redundant button role.
VideoShortsModal.tsx Documents intentional carousel interactions.
format.tsx Preserves keyboard-focusable tooltip marker.
ContractAdminPage.tsx Labels the actions column header.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontends/ol-components/src/components/Card/Card.tsx Outdated
Comment thread frontends/ol-components/src/components/Card/Card.tsx Outdated
Comment thread frontends/ol-components/src/components/NavDrawer/NavDrawer.tsx Outdated
The comment claimed the Escape listener was the only keyboard way out of
the drawer. It is not: the Close Navigation button at NavDrawer.tsx:204
is the primary exit, and this listener adds the Escape shortcut on top of
it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FyJ72HRTyQf6jVYmw3GZSt
The comments claimed the container click always forwards to the card's
anchor. It only does when forwardClicksToLink is set, and that defaults
to false (Card.tsx:321) -- in the other mode handleClick is the raw
onClick and the container is pointer-only.

Every caller today pairs the two (BaseLearningResourceCard), but nothing
enforces it and Card is an ol-components export. Comment says so now;
#3874 tracks making it impossible to get wrong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FyJ72HRTyQf6jVYmw3GZSt
@blarghmatey
blarghmatey merged commit 3fd0458 into main Aug 31, 2026
15 checks passed
@blarghmatey
blarghmatey deleted the a11y-styled-components-2-2 branch August 31, 2026 22:14
@odlbot odlbot mentioned this pull request Sep 1, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants