fix(a11y): make artwork lightbox triggers and interactive rows keyboard-accessible#69
Draft
Copilot wants to merge 4 commits into
Draft
fix(a11y): make artwork lightbox triggers and interactive rows keyboard-accessible#69Copilot wants to merge 4 commits into
Copilot wants to merge 4 commits into
Conversation
Copilot
AI
changed the title
[WIP] Fix accessibility for artwork lightbox triggers
fix(a11y): make artwork lightbox triggers and interactive rows keyboard-accessible
May 19, 2026
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 11 medium 5 high |
🟢 Metrics 6 complexity · -2 duplication
Metric Results Complexity 6 Duplication -2
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
… keyboard-accessible
InstaZDLL
force-pushed
the
copilot/fix-accessibility-lightbox-triggers
branch
from
c341b8e to
fdee4d5
Compare
github-actions
Bot
added
scope: frontend
Follow-up to Copilot's a11y pass on this branch. Codacy raised two
classes of findings:
1. Ten MEDIUM "JSX attribute is specific to React" — PMD running on
`.tsx`/`.jsx` and flagging `className` as if it were JSP. PMD
targets Java / Apex / PLSQL / JSP — none of which exist in this
repo. Drop PMD (and the Java runtime it pulled in) from
`.codacy/codacy.yaml`; eslint already covers JS/TS, Opengrep +
Trivy cover security, Lizard covers complexity.
2. Nine HIGH `jsx-a11y/*` findings — `<li>` / `<div>` non-interactive
elements carrying `tabIndex` + `role="button"`. Split the fix two
ways:
- Lyrics rows (`LyricsPanel`, `FullscreenLyrics`) and the now-
playing `QueueRow` (interactive variant was dead code anyway):
refactor into real `<button>` elements. `onKeyDown` becomes
unnecessary — `<button>` natively handles Enter and Space. Net
code reduction.
- `SortableQueueRow` and the six `TrackTable` rows: contain inner
`<button>` elements (drag handle, like, more-options), so the
outer can't be a `<button>` without producing nested-button
invalid HTML. Keep the `<div tabIndex role="button">` pattern
with bare `// eslint-disable-next-line` + comment explaining
why the compound is intentional; keyboard activation still
works end-to-end.
The bare disable directives target jsx-a11y rules that Codacy runs
server-side but that we don't load locally (no
`eslint-plugin-jsx-a11y` dep). Set linterOptions.reportUnusedDisableDirectives
to "off" in `eslint.config.js` so eslint v9 doesn't flag them as
unused locally.
Codacy runs its a11y checks via a cloud-side analyzer that doesn't read the repo's eslint config and doesn't honor inline `eslint-disable` directives. The seven bare `// eslint-disable-next-line` lines added in the previous commit were therefore load-bearing for nothing and only produced "Unused eslint-disable directive" warnings locally. Drop them along with the matching `reportUnusedDisableDirectives: "off"` opt-out in `eslint.config.js`. The explanatory comments above each compound row stay — they still document why the `<div tabIndex role="button">` pattern is intentional for rows that contain inner buttons.
The Codacy CLI install script leaked into the previous commit despite being declared in `.codacy/.gitignore` — that file is itself untracked, so its rules only apply on machines that already have it. Move the exclusion rules to the tracked root `.gitignore` so a fresh checkout doesn't accumulate the same machine-local artefacts.
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.
Summary
NowPlayingPanel,AlbumDetailView,ArtistDetailView) were bare<div onClick>/<img onClick>— not focusable, not announced by screen readers, and broken foruseModalA11yfocus restoration (capturedactiveElementwas<body>, not the trigger).cursor-pointerpatterns found the same gap on lyric-seek rows, queue rows, WrappedView artist rows, and all six track-table views.Artwork triggers (
NowPlayingPanel,AlbumDetailView,ArtistDetailView):<button type="button">+aria-label={t("common.viewArtwork")}+focus-visible:ring-2 focus-visible:ring-emerald-500ArtistDetailView:pointer-events-noneoverlay +pointer-events-autoedit-pencil preserved; pencil is a sibling of the new button, not nested inside it, so clicks don't cross-fireuseModalA11yround-trip now works:document.activeElementis the<button>on open → focus returns to it on closeLyrics seek rows (
LyricsPanel,FullscreenLyrics):role="button",tabIndex={0},onKeyDown(Enter/Space → seek) to synced<li>elementsWrappedView top-artists:
<li onClick>→<li><button>, matching the already-accessible top-albums pattern in the same componentQueuePanel:
QueueRow+SortableQueueItem:tabIndex,role="button",onKeyDown(Enter/Space → jump to track)Track rows (
LibraryView,PlaylistView,LikedView,AlbumDetailView,ArtistDetailView,GenreDetailView):tabIndex={0}+onKeyDown(Enter/Space → play); focus ring usesfocus-visible:ring-inset focus-visible:ring-emerald-500i18n:
common.viewArtworkadded to all 17 locale files.How I tested
bun run lintandbun run typecheckboth pass cleanlyArtistDetailViewstill opens the image picker independently of the lightbox buttonScreenshots / clips
Checklist
type(scope): subject, kebab-case scope)bun run lint+bun run typecheckpass locallycargo check --manifest-path src-tauri/Cargo.toml --all-targetspasses locallysrc/i18n/locales/updatedCLAUDE.mdanddocs/updated