Skip to content

Commit 0a8efda

Browse files
edenoclaude
andcommitted
refactor(a11y): apply Phase 10 review findings
code-reviewer / pr-test-analyzer / ux-reviewer: Contrast (completes the token migration): - Replace the remaining hardcoded #2196f3 literals across the new-page stylesheets with var(--color-primary) (now #1565c0) so primary buttons/borders actually meet AA — the contrast test guarded the token but components bypassed it. Update the stale contrast comment block in AnimalEditorStepper.scss. A11y (ux-reviewer must-fixes): - ?-opened help now focuses the header trigger first, so the shared Modal returns focus there on close (no dropped focus). - DayEditor form inputs keep a visible :focus-visible outline (was outline:none). - AnimalEditor Alt+N now opens the add-electrode-group dialog (was a silent no-op while the help advertised it); aria-atomic on its step announcer; drop redundant role="navigation". - ElectrodeGroups table: status badge is now an aria-labelled icon (not emoji-only, WCAG 1.4.1) with a stable class key; Edit buttons get a disambiguating aria-label. - Shortcuts trigger: title tooltip ("press ?") + :focus-visible ring. Tests (pr-test-analyzer gaps): - AnimalEditor stepper Alt+Arrow + Alt+N + aria-current; Alt+N no-op on a no-add step; shortcuts ignored while the help dialog is open; saveNow success + error paths. Document the jsdom <details> keyboard-activation limitation inline. Deferred (overview.md): Alt+Arrow vs browser Back/Forward on Windows/Linux. Baselines 125/125; full suite 3006 passed / 0 skipped; lint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 739160c commit 0a8efda

23 files changed

Lines changed: 225 additions & 102 deletions

‎.claude/docs/plans/v3-workspace-cutover/overview.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,13 @@ not executed here.
229229
relative/human-readable day labels for animals with 60–200+ days, and an explicit
230230
success confirmation. Deferred to the [Phase 10](phase-10-a11y-keyboard.md) pass.
231231

232+
- **Phase 10 review item deferred.** The `Alt+←` / `Alt+→` stepper shortcuts (the
233+
chord the phase spec prescribed) collide with the browser's Back/Forward navigation
234+
on Windows/Linux; the handler `preventDefault`s, so in-app it advances the stepper
235+
rather than navigating history. **Follow-up:** consider `Alt+PageUp/PageDown` (or
236+
`Alt+Shift+Arrow`) for cross-platform safety, or add a platform note in the shortcuts
237+
help. Kept as specified for now; revisit before v3.0.0 if user feedback warrants.
238+
232239
## Plan revisions
233240

234241
- **2026-06-03 — parity model corrected; a byte-parity phase inserted after export.** Pre-Phase-5

‎src/__tests__/integration/aria-landmarks.test.jsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ describe('ARIA Landmarks', () => {
216216
expect(container.querySelectorAll('[role="banner"], header')).toHaveLength(1);
217217
expect(container.querySelectorAll('[role="contentinfo"], footer')).toHaveLength(1);
218218
expect(screen.getByRole('link', { name: /back to workspace/i })).toBeInTheDocument();
219+
// Exactly one step is marked current for assistive tech.
220+
expect(container.querySelectorAll('[aria-current="step"]')).toHaveLength(1);
219221
});
220222

221223
it('keeps the default route (#/) on the legacy form even with flags enabled', () => {

‎src/__tests__/integration/keyboard-navigation.test.jsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,10 @@ describe('Keyboard Navigation Accessibility', () => {
198198
firstSummary.focus();
199199
expect(document.activeElement).toBe(firstSummary);
200200

201-
// Activating the summary (Enter/click on a native disclosure) expands it.
201+
// Activating the summary expands it. jsdom does not toggle <details> on a
202+
// synthetic Enter/Space keydown (it implements activation only via click), so
203+
// we assert keyboard *focusability* above and activation via click here; the
204+
// real Enter/Space toggle is browser-native and covered by the e2e lane.
202205
await user.click(firstSummary);
203206
await waitFor(() => expect(details.open).toBe(true));
204207
});

‎src/__tests__/integration/shortcuts.integration.test.jsx‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,46 @@ describe('global shortcuts + help (integration)', () => {
8181
fireEvent.keyDown(document.body, { key: 'n', altKey: true });
8282
expect(await screen.findByRole('dialog', { name: /add task/i })).toBeInTheDocument();
8383
});
84+
85+
it('Alt+N is a no-op on a step with no add target (Overview)', async () => {
86+
await renderRoute(`#/day/${DAY_ID}`);
87+
await screen.findByRole('heading', { name: /session metadata/i });
88+
89+
fireEvent.keyDown(document.body, { key: 'n', altKey: true });
90+
await act(async () => { await Promise.resolve(); });
91+
// No dialog appears and the view is unchanged.
92+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
93+
expect(screen.getByRole('heading', { name: /session metadata/i })).toBeInTheDocument();
94+
});
95+
96+
it('further shortcuts are ignored while the help dialog is open', async () => {
97+
await renderRoute(`#/day/${DAY_ID}`);
98+
await screen.findByRole('heading', { name: /session metadata/i });
99+
100+
fireEvent.keyDown(document.body, { key: '?' });
101+
expect(await screen.findByRole('dialog', { name: /keyboard shortcuts/i })).toBeInTheDocument();
102+
103+
// With a modal open, Alt+N must not open another dialog and Alt+Arrow must not
104+
// navigate the stepper underneath.
105+
fireEvent.keyDown(document.body, { key: 'n', altKey: true });
106+
fireEvent.keyDown(document.body, { key: 'ArrowRight', altKey: true });
107+
await act(async () => { await Promise.resolve(); });
108+
expect(screen.getAllByRole('dialog')).toHaveLength(1);
109+
expect(screen.getByRole('dialog', { name: /keyboard shortcuts/i })).toBeInTheDocument();
110+
});
111+
112+
it('Alt+ArrowRight and Alt+N drive the AnimalEditor stepper', async () => {
113+
await renderRoute('#/animal/remy/editor');
114+
await screen.findByRole('heading', { name: /step 1: electrode groups/i });
115+
116+
// Alt+N on step 1 opens the add-electrode-group dialog.
117+
fireEvent.keyDown(document.body, { key: 'n', altKey: true });
118+
expect(await screen.findByRole('dialog', { name: /add electrode group/i })).toBeInTheDocument();
119+
fireEvent.keyDown(document, { key: 'Escape' });
120+
await act(async () => { await Promise.resolve(); });
121+
122+
// Alt+ArrowRight advances to Channel Maps.
123+
fireEvent.keyDown(document.body, { key: 'ArrowRight', altKey: true });
124+
expect(await screen.findByRole('heading', { name: /step 2: channel maps/i })).toBeInTheDocument();
125+
});
84126
});

‎src/components/CalendarDayCreator/CalendarDayCreator.css‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,16 @@
6565
padding: 0.375rem 0.75rem;
6666
font-size: 0.875rem;
6767
font-weight: 500;
68-
color: var(--color-primary, #2196f3);
68+
color: var(--color-primary, var(--color-primary));
6969
background-color: var(--color-white, #ffffff);
70-
border: 1px solid var(--color-primary, #2196f3);
70+
border: 1px solid var(--color-primary, var(--color-primary));
7171
border-radius: 4px;
7272
cursor: pointer;
7373
transition: background-color 0.2s, color 0.2s;
7474
}
7575

7676
.btn-today:hover {
77-
background-color: var(--color-primary, #2196f3);
77+
background-color: var(--color-primary, var(--color-primary));
7878
color: var(--color-white, #ffffff);
7979
}
8080

@@ -130,7 +130,7 @@
130130
}
131131

132132
.calendar-day:focus {
133-
outline: 2px solid var(--color-primary, #2196f3);
133+
outline: 2px solid var(--color-primary, var(--color-primary));
134134
outline-offset: 2px;
135135
}
136136

@@ -140,14 +140,14 @@
140140
}
141141

142142
.calendar-day--today {
143-
border: 2px solid var(--color-primary, #2196f3);
143+
border: 2px solid var(--color-primary, var(--color-primary));
144144
font-weight: 600;
145145
}
146146

147147
.calendar-day--selected {
148-
background-color: var(--color-primary, #2196f3);
148+
background-color: var(--color-primary, var(--color-primary));
149149
color: var(--color-white, #ffffff);
150-
border-color: var(--color-primary, #2196f3);
150+
border-color: var(--color-primary, var(--color-primary));
151151
}
152152

153153
.calendar-day--selected:hover {
@@ -221,7 +221,7 @@
221221
}
222222

223223
.legend-indicator--selected {
224-
background-color: var(--color-primary, #2196f3);
224+
background-color: var(--color-primary, var(--color-primary));
225225
}
226226

227227
.legend-indicator--existing {
@@ -230,7 +230,7 @@
230230
}
231231

232232
.legend-indicator--today {
233-
border: 2px solid var(--color-primary, #2196f3);
233+
border: 2px solid var(--color-primary, var(--color-primary));
234234
background-color: var(--color-white, #ffffff);
235235
}
236236

@@ -260,7 +260,7 @@
260260

261261
.btn-primary {
262262
color: var(--color-white, #ffffff);
263-
background-color: var(--color-primary, #2196f3);
263+
background-color: var(--color-primary, var(--color-primary));
264264
}
265265

266266
.btn-primary:hover:not(:disabled) {

‎src/components/ShortcutsHelp/ShortcutsHelp.scss‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,8 @@
5050
.shortcuts-trigger:hover {
5151
background: #f5f5f5;
5252
}
53+
54+
.shortcuts-trigger:focus-visible {
55+
outline: 2px solid var(--color-primary);
56+
outline-offset: 2px;
57+
}

‎src/layouts/AppLayout.jsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,16 @@ export function AppLayout() {
103103
// navigation / add are broadcast to whichever stepper is on screen; help opens a
104104
// dialog; Ctrl/Cmd+S flushes the workspace save.
105105
const [shortcutsOpen, setShortcutsOpen] = useState(false);
106+
const shortcutsTriggerRef = useRef(null);
107+
// Open help; focus the trigger first so that when `?` opens the dialog (no element
108+
// focused), the Modal captures a real opener and returns focus to it on close.
109+
const openShortcuts = () => {
110+
shortcutsTriggerRef.current?.focus();
111+
setShortcutsOpen(true);
112+
};
106113
useGlobalShortcuts({
107114
onSave: persistence.saveNow,
108-
onShowHelp: () => setShortcutsOpen(true),
115+
onShowHelp: openShortcuts,
109116
onNextStep: () => emitStepperShortcut('next'),
110117
onPrevStep: () => emitStepperShortcut('prev'),
111118
onAdd: () => emitStepperShortcut('add'),
@@ -204,11 +211,13 @@ export function AppLayout() {
204211
<img src={logo} alt="Loren Frank Lab logo" />
205212
</a>
206213
<button
214+
ref={shortcutsTriggerRef}
207215
type="button"
208216
className="shortcuts-trigger"
209217
onClick={() => setShortcutsOpen(true)}
210218
aria-label="Keyboard shortcuts"
211219
aria-haspopup="dialog"
220+
title="Keyboard shortcuts (press ?)"
212221
>
213222
<span aria-hidden="true">⌨</span>
214223
<span className="visually-hidden">Keyboard shortcuts</span>

‎src/pages/AnimalEditor/AnimalEditorStepper.jsx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,15 @@ export default function AnimalEditorStepper() {
7272
// early returns below to satisfy the Rules of Hooks; the step count is filled in
7373
// once `steps` is built further down (stepCountRef.current = steps.length).
7474
const stepCountRef = useRef(1);
75+
// The current step's "add" handler (Alt+N), or null when the step has no add
76+
// target. Filled in below once the handlers + active step are known.
77+
const addHandlerRef = useRef(null);
7578
useStepperShortcut(
7679
useCallback((action) => {
80+
if (action === 'add') {
81+
addHandlerRef.current?.();
82+
return;
83+
}
7784
setActiveStep((cur) => {
7885
if (action === 'next') return Math.min(cur + 1, stepCountRef.current - 1);
7986
if (action === 'prev') return Math.max(cur - 1, 0);
@@ -562,9 +569,11 @@ export default function AnimalEditorStepper() {
562569

563570
// Check if we're on the final step
564571
const isOnFinalStep = activeStep === steps.length - 1;
565-
// Keep the shortcut handler's step count current (the ref + hook are declared up
566-
// top, before the early returns, to satisfy the Rules of Hooks).
572+
// Keep the shortcut handler's step count + add target current (the ref + hook are
573+
// declared up top, before the early returns, to satisfy the Rules of Hooks). Alt+N
574+
// adds an electrode group on the Electrode Groups step (0); other steps have no add.
567575
stepCountRef.current = steps.length;
576+
addHandlerRef.current = activeStep === 0 ? handleAddGroup : null;
568577

569578
return (
570579
<div className="animal-editor-stepper">
@@ -582,7 +591,7 @@ export default function AnimalEditorStepper() {
582591
</div>
583592

584593
{/* Step indicators */}
585-
<nav className="animal-editor-step-nav" role="navigation" aria-label="Configuration steps">
594+
<nav className="animal-editor-step-nav" aria-label="Configuration steps">
586595
<ul className="step-indicators">
587596
{steps.map((step, index) => (
588597
<li
@@ -615,7 +624,7 @@ export default function AnimalEditorStepper() {
615624

616625
{/* Announce the active step to screen readers on change (matches the
617626
DayEditor route announcer pattern). */}
618-
<div className="visually-hidden" role="status" aria-live="polite">
627+
<div className="visually-hidden" role="status" aria-live="polite" aria-atomic="true">
619628
{`Step ${activeStep + 1} of ${steps.length}: ${steps[activeStep].label}`}
620629
</div>
621630

‎src/pages/AnimalEditor/AnimalEditorStepper.scss‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,22 +43,22 @@
4343
padding: 0.5rem 1rem;
4444
font-size: 0.875rem;
4545
font-weight: 500;
46-
color: #2196f3;
46+
color: var(--color-primary);
4747
background-color: transparent;
48-
border: 1px solid #2196f3;
48+
border: 1px solid var(--color-primary);
4949
border-radius: 4px;
5050
text-decoration: none;
5151
transition: background-color 0.2s ease, color 0.2s ease;
5252
white-space: nowrap;
5353
cursor: pointer;
5454

5555
&:hover {
56-
background-color: #2196f3;
56+
background-color: var(--color-primary);
5757
color: #fff;
5858
}
5959

6060
&:focus {
61-
outline: 2px solid #2196f3;
61+
outline: 2px solid var(--color-primary);
6262
outline-offset: 2px;
6363
}
6464
}
@@ -107,7 +107,7 @@
107107
}
108108

109109
&:focus {
110-
outline: 2px solid #2196f3;
110+
outline: 2px solid var(--color-primary);
111111
outline-offset: -2px;
112112
z-index: 1;
113113
}
@@ -135,13 +135,13 @@
135135
/* Active step */
136136
.step-indicator.active {
137137
.step-indicator-button {
138-
border-bottom-color: #2196f3;
138+
border-bottom-color: var(--color-primary);
139139
background-color: #e3f2fd;
140140
font-weight: 600;
141141
}
142142

143143
.step-number {
144-
background-color: #2196f3;
144+
background-color: var(--color-primary);
145145
color: #fff;
146146
}
147147
}
@@ -194,7 +194,7 @@
194194
font-family: inherit;
195195

196196
&:focus {
197-
outline: 2px solid #2196f3;
197+
outline: 2px solid var(--color-primary);
198198
outline-offset: 2px;
199199
}
200200

@@ -220,7 +220,7 @@
220220

221221
.btn-next,
222222
.btn-save {
223-
background-color: #2196f3;
223+
background-color: var(--color-primary);
224224
color: #fff;
225225

226226
&:hover:not(:disabled) {
@@ -277,7 +277,7 @@
277277
font-family: inherit;
278278

279279
&:focus {
280-
outline: 2px solid #2196f3;
280+
outline: 2px solid var(--color-primary);
281281
outline-offset: 2px;
282282
}
283283

@@ -288,7 +288,7 @@
288288
}
289289

290290
.btn-primary {
291-
background-color: #2196f3;
291+
background-color: var(--color-primary);
292292
color: #fff;
293293

294294
&:hover:not(:disabled) {
@@ -405,7 +405,7 @@ input[type="file"] {
405405
}
406406

407407
.step-indicator.active .step-indicator-button {
408-
border-left-color: #2196f3;
408+
border-left-color: var(--color-primary);
409409
}
410410

411411
.animal-editor-footer {
@@ -478,11 +478,11 @@ input[type="file"] {
478478

479479
/* ===== WCAG AA Compliance Notes ===== */
480480
/*
481-
* Color contrast ratios verified:
482-
* - Primary blue (#2196f3) on white: 3.15:1 (AA for large text)
483-
* - Primary dark (#1976d2) on white: 4.58:1 (AA)
484-
* - Error red (#d32f2f) on white: 4.52:1 (AA)
485-
* - Success green (#2e7d32) on white: 4.56:1 (AA)
481+
* Color contrast ratios verified (see src/__tests__/unit/a11y/contrast.test.js):
482+
* - Primary (--color-primary #1565c0) on white: 5.75:1 (AA)
483+
* - Warning (--color-warning #bf360c) on white: 5.60:1 (AA)
484+
* - Error (--color-error #c62828) on white: 5.62:1 (AA)
485+
* - Success green (#2e7d32) on white: 5.13:1 (AA)
486486
* - Grey 600 (#666) on white: 5.74:1 (AA)
487487
* - Grey 900 (#333) on white: 12.6:1 (AAA)
488488
*

0 commit comments

Comments
 (0)