WebUI: accessibility pass to WCAG 2.1 AA, and announce a finished run once (#598, #599) - #744
Merged
Conversation
Accessibility (#598), the last open item under #599: - Keyboard. Add a skip link past the sidebar's ten controls, and give the command palette and the config/profile picker real dialog semantics via a shared openModal(): Tab is trapped inside, Escape closes, and focus returns to the opener. The picker's entries were click-handler <li>s and so were not reachable by keyboard at all; they are now buttons. - Screen readers. Name the nav landmark and mark the active view with aria-current; expose the progress bar as a progressbar with live aria-valuenow/aria-valuetext; add one polite live region for SSE progress. That region is throttled (phase change, else at most every 15s) because progress arrives several times a second and an unthrottled region makes the page unusable. Associate every form label, scope every column header, hide decorative icons, and name the repeated row-action buttons. Each kind of information gets exactly one channel: the run pill is deliberately not a live region, since the toast stack already speaks completed/failed and a second region would say it twice. - Color. The light palette had genuine AA failures — --fg-faint at 2.73:1, the primary button at 3.39:1, the semantic badges at ~4.2:1 — and --border-strong failed 1.4.11 (3:1) in both themes as the boundary that identifies a text input. Corrected values were solved for along each color's own hue so the design shifts as little as possible. The PWA theme-color follows --accent. Also fixes the stacking completed-run toast surfaced during #596 verification: applyRunState is re-entered by the SSE terminal events, the run-pill poll, and every dashboard mount, and it announced on "status is terminal" rather than "status just became terminal", so one finished run stacked a fresh toast (and re-fired the desktop notification) on every refresh. Announcements now key on the observed transition, and a run that was already over when the page loaded stays silent. Verified in Chrome 151 over the DevTools Protocol: the computed accessibility tree resolves a non-empty name for every interactive control across all six views, Tab genuinely stays inside both modals, focus returns to the opener, aria-activedescendant tracks the palette selection, and the throttle and announce-once behavior hold against synthetic run states. contrast_test.go and a11y_test.go pin the same properties in CI, where no browser is available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Maturity section still described the remote/team-facing case as beta with "no login rate-limiting yet" and pointed at #599 for the remaining hardening. Both were stale: every gating item has landed — throttling (#594), session renewal and a 7-day cap (#601), loopback metrics-addr (#602), CSP without inline script (#603), soak coverage (#595), secret externalization (#597), trusted-proxy attribution (#604), and cross-browser verification (#596). Remote is now described as supported, with the single-operator shared-secret model stated as a v1 design decision rather than a gap, and the two honest caveats kept (Gecko unverified; no screen-reader run). Adds an Accessibility section covering the keyboard, screen-reader, color, and motion contracts, and a "Verifying in a real browser" runbook for driving the console over the Chrome DevTools Protocol — CI has no browser, so this is the method rather than a committed harness. It records the three traps that cost real time: --dump-dom returns the pre-boot DOM because the SPA mounts after an async login round-trip, --virtual-time-budget hangs forever because the SSE stream means the page never goes idle, and /json/new requires PUT on Chrome 111+. Under WSL, Windows-side Chrome works over localhost in both directions but --remote-debugging-pipe cannot cross the boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the embedded WebUI SPA by tightening accessibility semantics (WCAG 2.1 AA), preventing duplicate “run finished” announcements across re-entrant state updates, and updating docs/changelog to reflect the current supported deployment posture.
Changes:
- Adds keyboard and screen-reader accessibility improvements across the SPA (skip link, modal focus trapping, ARIA roles/labels/live regions, table header scoping, decorative icon hiding).
- Fixes run-end notifications/toasts to announce only on terminal-state transitions (deduped across SSE, polling, and mounts), and adds regression tests.
- Updates the theme/palette to meet contrast requirements and adds Go tests that pin contrast + a11y structure in CI; updates docs and changelog accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/webui/static/manifest.webmanifest | Updates PWA theme color to match the revised accent palette. |
| internal/webui/static/index.html | Keeps theme-color meta in sync with the CSS accent token. |
| internal/webui/static/app.js | Implements modal plumbing, live-region progress announcements, navigation ARIA, and run-end deduping. |
| internal/webui/static/app.css | Adjusts palette tokens for AA contrast and adds utilities (sr-only, skip link), plus minor border styling tweaks. |
| internal/webui/frontend_test.go | Adds an asset-shape regression test to guard run-end announcement deduping. |
| internal/webui/contrast_test.go | New: parses embedded CSS palette tokens and asserts WCAG contrast ratios + theme override consistency. |
| internal/webui/a11y_test.go | New: asserts key a11y structural requirements in the embedded JS/CSS source. |
| docs/WEBUI.md | Updates maturity/support stance and adds accessibility + real-browser verification guidance. |
| CHANGELOG.md | Documents the accessibility work, run-end announcement fix, and updated WebUI support posture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+517
to
+522
| function setProgressNow(pct, rows, total) { | ||
| const track = $("#pbar-track"); | ||
| if (!track) return; | ||
| track.setAttribute("aria-valuenow", String(pct)); | ||
| track.setAttribute("aria-valuetext", total ? `${pct}% — ${fmtNum(rows)} of ${fmtNum(total)} rows` : `${pct}%`); | ||
| } |
setProgressNow() stated a total whenever one was passed, and the completed-run
path passed rows_transferred as both the count and the total. That announced
"100% — 3,000,000 of 3,000,000 rows" for a finished run, asserting a
denominator that was never measured — the run reports what it transferred, not
what it was measured against.
Fixed at the call site rather than in the helper: the completed path now passes
no total, and the helper grew a "count known, total unknown" case so the tally
is still announced ("100% — 3,000,000 rows").
Copilot suggested treating total === rows as "no total available", but that
heuristic misfires on a legitimate case: a mid-run progress tick reaches
rows === rows_total at the end of a transfer, where the total is real and worth
saying. Deciding this in the helper by comparing values would suppress a true
total on a coincidence; only the caller knows whether it has a measured
denominator.
Verified in Chrome across all three cases: mid-run with a real total still
reads "42% — 1,200,000 of 3,000,000 rows", a mid-run tick at genuine 100% still
reads "100% — 3,000,000 of 3,000,000 rows", and a completed run now reads
"100% — 3,000,000 rows".
Also records two more traps in the browser-verification runbook, both of which
produced a false reading while checking this fix: the service worker serves a
cached app.js keyed on the dmt version, so an unchanged version hides asset
edits from the page; and a second Chrome cannot bind an already-held debug
port, so CDP silently attaches to the previous instance.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #598. Closes #599.
This finishes the last open item under the #599 release-readiness tracker, fixes an untracked bug found during #596 verification, and brings the docs in line with what has actually shipped.
Accessibility (#598)
Keyboard. A skip link jumps past the sidebar's ten controls. The command palette and the config/profile picker are now real modal dialogs sharing one
openModal(): Tab is trapped inside, Escape closes, focus returns to the opener. The picker's entries were click-handler<li>s — not reachable by keyboard at all — and are now buttons.Screen readers. Named nav landmark with
aria-current; the progress bar is aprogressbarwith livearia-valuenow/aria-valuetext; one polite live region for SSE progress, throttled to a phase change or at most every 15s (progress arrives several times a second — unthrottled makes the page unusable). Every form label associated, every column header scoped, decorative icons hidden, repeated row-action buttons named.Each kind of information gets exactly one channel: the run pill is deliberately not a live region, because the toast stack already speaks completed/failed and a second region would say it twice.
Color. The light palette had real AA failures —
--fg-faintat 2.73:1, the primary button at 3.39:1, semantic badges at ~4.2:1 — and--border-strongfailed 1.4.11 (3:1) in both themes as the boundary that identifies a text input. Corrected values were solved for along each color's own hue, so the design shifts as little as possible.Announce-once run state (#599)
applyRunStateis re-entered by the SSE terminal events, the run-pill poll, and every dashboard mount. It announced on "status is terminal" rather than "status just became terminal", so one finished run stacked a fresh toast — and re-fired the desktop notification — on every refresh. Announcements now key on the observed transition; a run already over at page load stays silent.Docs
Maturity said remote/team-facing was beta with "no login rate-limiting yet" and pointed at #599 for remaining hardening. Both stale — every gating item has landed (#594, #601, #602, #603, #595, #597, #604, #596). Now described as supported, with the single-operator shared-secret model stated as a v1 design decision rather than a gap. Caveats kept honest: Gecko unverified, no screen-reader run.
Also adds a "Verifying in a real browser" runbook for driving the console over CDP, including the three traps that cost real time (
--dump-domreturns the pre-boot DOM;--virtual-time-budgethangs forever on the open SSE stream;/json/newneedsPUTon Chrome 111+).Verification
Driven in Chrome 151 over the DevTools Protocol — not just asserted against source:
aria-activedescendanttracks the palette selectioncontrast_test.goanda11y_test.gopin the same properties in CI, where no browser is available. The contrast test was negative-tested: reverting one token fails the build with the measured ratio.Full suite green (37 packages).
Not covered
Both are recorded in
docs/WEBUI.mdrather than left implied.🤖 Generated with Claude Code