Repository navigation
fix(dashboard): announce transcript, mark active nav, real modal behaviour (A11Y-1/4/5 + latch) - #15
Merged
Merged
Conversation
The double-submit guards in EditAgentDrawer were hand-rolled inline refs, so they could only be exercised by mounting the 1400-line drawer - whose mount effects never settle in jsdom, making a component-level regression test impractical. Extract runExclusive(ref, fn) into asyncLatch.ts and route both save handlers through it. confirmListRename keeps its validation outside the latch, so a rejected name can no longer leave the guard set. Behaviour is unchanged; the guard is now covered by asyncLatch.test.ts, which fails if the ref check is removed.
…1Y-1, A11Y-4) A11Y-1: the chat transcript was not a live region, so a screen reader user got no announcement when a reply arrived and had to re-navigate the log manually. Both render paths (plain div and the virtualised Virtuoso list) now carry role="log", aria-live="polite" and aria-relevant="additions", with aria-busy bound to isStreaming so announcements are not fired per streamed token. A11Y-4: the active sidebar entry was conveyed by colour alone. Both the expanded row and the collapsed icon rail now set aria-current="page". The collapsed rail also gains aria-label: its text lives in an antd Tooltip, which is not part of the accessibility tree, so each button announced as "button".
…(A11Y-5) The three hand-rolled overlays (PwaInstallPrompt's IosGuide and DesktopInstallGuide, ServiceRestartOverlay) already declared role="dialog"/"alertdialog" with aria-modal="true", but none implemented it: focus stayed on the trigger behind the overlay, Tab walked into the page the attribute had declared inert, Escape did nothing, and closing did not restore focus. The markup promised modality that the keyboard never got. Add AppDialog: focus moves to the first control on open, Tab and Shift+Tab wrap inside, Escape dismisses when dismissable, and focus returns to the trigger on close. Layout and styling stay with the caller. ServiceRestartOverlay is dismissable only in its confirm and timeout phases — Escape must not close it out from under a user mid-restart or on the success screen. AppDialog.test.tsx covers all six behaviours and fails if the Tab trap or the focus restore is removed.
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.
Fork remediation slice. Four commits, 20 files.
Commits
75b0c5aaae098e12aria-currentcf2178e1AppDialogwith real modal keyboard behaviourf9ffd84dFE-5/FE-12 — the latch
The double-submit guards were hand-rolled inline refs, exercisable only by
mounting a 1400-line drawer whose mount effects never settle in jsdom. Extracted
runExclusive(ref, fn)and routed both handlers through it.confirmListRenamekeeps its validation outside the latch so a rejected name cannot leave the
guard set.
A11Y-1 — the transcript is a live region
It was not, so a screen reader user got no announcement when a reply arrived.
Both render paths now carry
role="log",aria-live="polite",aria-relevant="additions", witharia-busybound toisStreamingsoannouncements are not emitted per streamed token.
A11Y-4 —
aria-currenton the sidebarZero
aria-currentunderlayouts/before the fix. Both nav surfaces now setit. Also found: the collapsed rail's buttons had no accessible name at all —
the label lived only in an antd Tooltip, which is not in the accessibility tree,
so every icon button announced as "button". Added
aria-label.A11Y-5 —
AppDialogThe three hand-rolled overlays already declared
role="dialog"/"alertdialog"with
aria-modal="true", but implemented none of it — zero Escape/keydownhandlers in any of them. Focus stayed on the trigger behind the overlay, Tab
walked into the page the attribute had declared inert, Escape did nothing, and
closing did not restore focus.
AppDialogprovides focus-in, Tab/Shift+Tab wrap, Escape and focus restore;layout and styling stay with the caller, so appearance is unchanged.
ServiceRestartOverlayis dismissable only inconfirm/timeout— Escape mustnot close it out from under a user mid-restart.
A11Y-6 — the default palette failed AA
The contrast gate already existed, but deliberately exempted rose — and rose
is the default palette. Measured across all 16 primary tokens: rose was the only
one failing, at 3.36:1 light / 2.37:1 dark against the 4.5:1 floor.
Now
#B04759with darker hover/active, which also brings rose in line with theconvention every other palette already follows (same value in both modes, hover
and active go darker). Dark mode keeps a separate bright
#E8899Afor text/linkroles — button background and on-dark text need opposite values, which is why one
shared hex could not work. 30 tint sites plus the boot spinner, offline page and
token fallbacks follow.
New
themeContrast.test.tspins all 8 palettes and asserts the default paletteis covered, so the exemption cannot be reintroduced silently.
Test plan
cd dashboard && npx tsc -b→ cleannpx eslinton changed files → 0 errorsmake allgreen on each commit via the pre-commit hookasyncLatch.test.ts(5),MessageList.a11y.test.tsx(3),SidebarCollapsedIconNav.a11y.test.tsx(3),AppDialog.test.tsx(6),themeContrast.test.ts(10)Every new guard is mutation-verified — the suite fails when the behaviour is
removed:
runExclusivearia-currentfrom the collapsed railrole="log"siteAppDialogcase fails#E85D75for rosethemeContrastfails with the 3.36 figureNotes / known limitations
MessageList's virtualised branch is asserted by source-attribute count, notby rendering — mounting Virtuoso with a real message list does not settle in
jsdom (it hangs rather than failing). The plain path is asserted by rendering.
Weaker than a render assertion; flagged in the test comment.
not added. It would fail emerald (2.54:1) and amber (2.15:1) for a rule the
design does not rely on — the chips are
aria-hiddenfills inside a labelled42px button. The test documents the reasoning and the measured spread instead.
(bare
<span>, noaria-hidden, no text alternative, at three sites inAgentSelector.tsx) and still use the saturated red/green pair. Needslightness/shape as well as hue, plus an accessible name.
AuthGuard,DocumentPreviewCore.docxSanitize,SkillDrawer,skillMarkdown,useChat.history) are load-order dependent and not caused by this branch.Neither CI nor
make allruns vitest.