Skip to content

Commit 53060ad

Browse files
edenoclaude
andcommitted
Pre-cutover cleanup: dialog-on-Modal migrations, alertdialog, contrast, behavioral-events relabel, reconfig versioning
Finish the deferred accessibility/correctness follow-ups before the v3 cutover. YAML export is unchanged — golden baselines stay byte-identical (125/125). - Migrate ChannelMapEditor, CopyFromAnimalDialog, and CalendarDayCreator onto the shared <Modal> primitive (focus trap, focus return, ESC/overlay close, scroll lock, aria-modal, labelled title). CopyFromAnimalDialog stops using a non-trapping <dialog open>; bespoke overlay markup and the dead dialog.electrode-group-modal styles are removed; CopyFromAnimalDialog body styles re-homed to its own SCSS. - ConfirmDialog renders role="alertdialog" (message wired via aria-describedby) and no longer dismisses on backdrop click when destructive; routine confirms unchanged. - Fix CalendarDayCreator contrast: muted text and adjacent-month day numbers move to the --color-grey-600 token (>=4.5:1 on white, hover grey, off-white legend); fix the re-homed copy-preview text to --color-primary. Add the audited pairs to contrast.test.js. - Relabel inherited behavioral events as animal-level reference that is NOT written to this day's export (only day-specific events are exported). mergeDayMetadata unchanged; a merge-invariant test locks that animal events are never concatenated in. - addConfigurationSnapshot returns the created version (from authoritative store state); ReconfigWizard applies forward to that returned version instead of re-deriving it, removing the cross-action desync/orphan-snapshot risk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 69e485b commit 53060ad

29 files changed

Lines changed: 755 additions & 303 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# v3.0.0 Workspace Cutover — Implementation Plan
22

3-
**Status:** Phases 0–10 complete (merged to `modern`). Phase 11 (cutover) next.
3+
**Status:** Phases 0–10.5 complete (merged to `modern`). Phase 11 (cutover) next.
44

55
Carries the `modern` branch from its mid-build state (M8a complete; new multi-page workspace exists
66
but cannot export YAML and loses data on refresh) through to **v3.0.0**, where the workspace UI is a

‎.claude/docs/plans/v3-workspace-cutover/post-v3-followups.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,40 @@ Track these so they aren't lost once the plan's numbered phases are done.
3737
logs via `console.error`; route these through a real logging path (error IDs / Sentry-style) once such
3838
infrastructure exists. Cross-app concern, not specific to one page.
3939

40+
## Surfaced by the pre-cutover dialog migration (reviewer findings, out of that scope)
41+
42+
These are pre-existing issues that the dialog-on-`<Modal>` migration made more visible (a
43+
focus-trapped modal raises the stakes of keyboard/AT gaps in the enclosed content). They were
44+
out of scope for the cleanup pass and are tracked here.
45+
46+
7. **CalendarDay grid is keyboard-unreachable when viewing a non-current month.**
47+
[CalendarDay.jsx](../../../../src/components/CalendarDayCreator/CalendarDay.jsx) uses
48+
`tabIndex={isToday ? 0 : -1}`, so once the user navigates to a month that does not contain
49+
"today", every day cell has `tabIndex=-1` and Tab skips the whole grid — and now that the
50+
calendar is a focus-trapped modal, a keyboard-only user cannot select an off-month date at all
51+
(only ESC/nav/action buttons are reachable). Implement a proper roving tabindex that defaults to
52+
the first selectable cell of the displayed month when today is absent.
53+
54+
8. **CalendarGrid presents all 42 day cells as a single `role="row"`.**
55+
[CalendarGrid.jsx](../../../../src/components/CalendarDayCreator/CalendarGrid.jsx) wraps the 42
56+
cells in one row under `role="grid"`; AT grid-navigation announces one row of 42 columns instead
57+
of 6 weeks × 7 days. Split into one `role="row"` per week (chunks of 7).
58+
59+
9. **ChannelMapEditor empty-state instruction is a dead end.**
60+
[ChannelMapEditor.jsx](../../../../src/pages/AnimalEditor/ChannelMapEditor.jsx) says "Please
61+
auto-generate channel maps first," but there is no auto-generate affordance; maps are generated
62+
implicitly when an electrode group is saved with a device type. Reword to point the user at the
63+
real action (close the editor, re-save the electrode group). Pre-existing copy, now shown in a
64+
more prominent focused dialog.
65+
66+
10. **`addConfigurationSnapshot` return value diverges from the assigned version on multiple adds
67+
within one render tick.** The returned version derives from `workspaceRef.current` (authoritative
68+
*now*), while the in-updater assignment uses `prev.length + 1`; for a single create-then-apply per
69+
tick (the only caller, the reconfig wizard) they agree, but two adds in the same tick would both
70+
return the same number while assigning sequential ones. Not reachable today; revisit if another
71+
caller batches snapshot creation. See
72+
[useWorkspace.js](../../../../src/state/useWorkspace.js) `addConfigurationSnapshot`.
73+
4074
## Release-gated
4175

4276
6. **Persistence-blob forward migration.** [Phase 1](phase-1-persistence.md) versions the localStorage

‎docs/REFACTOR_CHANGELOG.md‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,49 @@
22

33
**Purpose:** Track all changes made during the refactoring milestones.
44

5-
**Last Updated:** June 3, 2026
5+
**Last Updated:** June 4, 2026
6+
7+
---
8+
9+
## Pre-cutover cleanup (June 4, 2026) ✅ COMPLETE
10+
11+
### Summary
12+
13+
A grab-bag of deferred accessibility and correctness fixes so the cutover lands on a
14+
clean base: finish migrating the last bespoke dialogs onto the shared `<Modal>`, give
15+
destructive confirms the right role, close one color-contrast gap, clarify how inherited
16+
behavioral events relate to a day's export, and make probe-reconfiguration versioning
17+
atomic. **YAML export is unchanged — golden baselines stay byte-identical.**
18+
19+
### Changes
20+
21+
- **Last dialogs on the shared `<Modal>`.** `ChannelMapEditor`, `CopyFromAnimalDialog`,
22+
and `CalendarDayCreator` now render through the shared `<Modal>` primitive instead of
23+
hand-rolled overlays (`CopyFromAnimalDialog` previously used a non-trapping
24+
`<dialog open>`). They inherit the primitive's focus trap, focus return, Esc/overlay
25+
close, and scroll lock; their bespoke overlay markup and the now-dead
26+
`dialog.electrode-group-modal` styles were removed. A parameterized integration test
27+
asserts trap + focus-return + Esc per dialog, and the Axe suite now opens each one.
28+
- **`role="alertdialog"` for destructive confirms.** `ConfirmDialog` passes
29+
`role="alertdialog"` (with the message wired via `aria-describedby`) when `destructive`,
30+
so delete confirmations are announced as alerts; routine confirms stay `role="dialog"`.
31+
- **CalendarDayCreator contrast.** Muted secondary text and adjacent-month day numbers
32+
moved off low-contrast literals (`#757575` on the off-white legend was 4.41:1;
33+
other-month numbers were `#bdbdbd` at 1.88:1) to the `--color-grey-600` token, which
34+
stays ≥4.5:1 on white, the hover grey, and the off-white legend. Both pairs were added
35+
to `contrast.test.js`.
36+
- **Inherited behavioral events clarified (no export change).** The Day Editor showed the
37+
animal's `behavioral_events` as "inherited" in a way that implied they were part of the
38+
day's export. They are not: `mergeDayMetadata` emits only the day's `behavioral_events`.
39+
The display now states the inherited list is animal-level reference that is *not written
40+
to this day's metadata* — only day-specific events are exported. `mergeDayMetadata` is
41+
unchanged; a new test locks that animal-level events are never concatenated into the
42+
export, so golden baselines remain byte-identical.
43+
- **Atomic reconfiguration versioning.** `addConfigurationSnapshot` now returns the
44+
created version number (from the authoritative store state), and the reconfiguration
45+
wizard applies the snapshot forward to that exact returned version instead of
46+
re-deriving it from a possibly-stale `animal` prop — removing the cross-action
47+
desync / orphan-snapshot risk. No store public-API keys changed.
648

749
---
850

‎src/__tests__/integration/axe-a11y.test.jsx‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import { axe } from 'jest-axe';
1313
import { App } from '../../App';
1414
import { StoreProvider } from '../../state/StoreContext';
1515
import { makeConfiguredWorkspace } from '../helpers/test-fixtures';
16+
import ChannelMapEditor from '../../pages/AnimalEditor/ChannelMapEditor';
17+
import CopyFromAnimalDialog from '../../pages/AnimalEditor/CopyFromAnimalDialog';
18+
import { CalendarDayCreator } from '../../components/CalendarDayCreator/CalendarDayCreator';
1619

1720
const workspace = makeConfiguredWorkspace();
1821
const ANIMAL_ID = 'remy';
@@ -96,4 +99,57 @@ describe('axe-a11y (configured workspace, all routes)', () => {
9699
await expectNoViolations(container);
97100
});
98101
});
102+
103+
// The overlay surfaces migrated onto the shared Modal primitive: rendered open so
104+
// Axe sees the live dialog (role, labelling, focusables), one case per dialog.
105+
describe('migrated dialogs (open) have no violations', () => {
106+
it('ChannelMapEditor', async () => {
107+
const { container } = render(
108+
<ChannelMapEditor
109+
electrodeGroup={{ id: 'eg1', device_type: 'tetrode_12.5', location: 'CA1' }}
110+
channelMaps={[
111+
{
112+
electrode_group_id: 'eg1',
113+
ntrode_id: '0',
114+
electrode_id: 0,
115+
bad_channels: [],
116+
map: { 0: 0, 1: 1, 2: 2, 3: 3 },
117+
},
118+
]}
119+
onSave={() => {}}
120+
onCancel={() => {}}
121+
/>
122+
);
123+
await screen.findByRole('dialog', { name: /channel map editor/i });
124+
await expectNoViolations(container);
125+
});
126+
127+
it('CopyFromAnimalDialog', async () => {
128+
const { animals } = workspace;
129+
const { container } = render(
130+
<CopyFromAnimalDialog
131+
open
132+
currentAnimalId="not-this-one"
133+
animals={animals}
134+
onCopy={() => {}}
135+
onCancel={() => {}}
136+
/>
137+
);
138+
await screen.findByRole('dialog', { name: /copy electrode groups/i });
139+
await expectNoViolations(container);
140+
});
141+
142+
it('CalendarDayCreator', async () => {
143+
const { container } = render(
144+
<CalendarDayCreator
145+
animalId={ANIMAL_ID}
146+
existingDays={[]}
147+
onCreateDays={() => {}}
148+
onClose={() => {}}
149+
/>
150+
);
151+
await screen.findByRole('dialog', { name: /recording days calendar/i });
152+
await expectNoViolations(container);
153+
});
154+
});
99155
});
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* Accessibility contract for the overlay surfaces migrated onto the shared `<Modal>`
5+
* primitive: ChannelMapEditor, CopyFromAnimalDialog, and CalendarDayCreator. Each
6+
* must open as a real `role="dialog"` (aria-modal), trap Tab/Shift-Tab focus, return
7+
* focus to its opener on close, and close on Escape — the behaviors the primitive
8+
* owns. One parameterized case per dialog.
9+
*/
10+
import { useState } from 'react';
11+
import { describe, it, expect, vi } from 'vitest';
12+
import { render, screen, waitFor } from '@testing-library/react';
13+
import userEvent from '@testing-library/user-event';
14+
import ChannelMapEditor from '../../pages/AnimalEditor/ChannelMapEditor';
15+
import CopyFromAnimalDialog from '../../pages/AnimalEditor/CopyFromAnimalDialog';
16+
import { CalendarDayCreator } from '../../components/CalendarDayCreator/CalendarDayCreator';
17+
18+
const DIALOGS = [
19+
{
20+
name: 'ChannelMapEditor',
21+
accessibleName: /channel map editor/i,
22+
render: (onClose) => (
23+
<ChannelMapEditor
24+
electrodeGroup={{ id: 'eg1', device_type: 'tetrode_12.5', location: 'CA1' }}
25+
channelMaps={[
26+
{
27+
electrode_group_id: 'eg1',
28+
ntrode_id: '0',
29+
electrode_id: 0,
30+
bad_channels: [],
31+
map: { 0: 0, 1: 1, 2: 2, 3: 3 },
32+
},
33+
]}
34+
onSave={() => {}}
35+
onCancel={onClose}
36+
/>
37+
),
38+
},
39+
{
40+
name: 'CopyFromAnimalDialog',
41+
accessibleName: /copy electrode groups/i,
42+
render: (onClose) => (
43+
<CopyFromAnimalDialog
44+
open
45+
currentAnimalId="a1"
46+
animals={{
47+
a1: {
48+
subject: { subject_id: 'a1' },
49+
devices: { electrode_groups: [], ntrode_electrode_group_channel_map: [] },
50+
},
51+
a2: {
52+
subject: { subject_id: 'a2' },
53+
devices: {
54+
electrode_groups: [{ id: '1', device_type: 'tetrode_12.5', location: 'CA1' }],
55+
ntrode_electrode_group_channel_map: [],
56+
},
57+
},
58+
}}
59+
onCopy={() => {}}
60+
onCancel={onClose}
61+
/>
62+
),
63+
},
64+
{
65+
name: 'CalendarDayCreator',
66+
accessibleName: /recording days calendar/i,
67+
render: (onClose) => (
68+
<CalendarDayCreator
69+
animalId="a1"
70+
existingDays={[]}
71+
onCreateDays={() => {}}
72+
onClose={onClose}
73+
/>
74+
),
75+
},
76+
];
77+
78+
/**
79+
* Harness with an opener button so focus-return has a real element to return to.
80+
* Starts closed; the test focuses + clicks the opener to open the dialog.
81+
* @param {object} props
82+
* @param {(onClose: Function) => JSX.Element} props.renderDialog - Renders the dialog.
83+
* @returns {JSX.Element}
84+
*/
85+
function Harness({ renderDialog }) {
86+
const [open, setOpen] = useState(false);
87+
return (
88+
<>
89+
<button type="button" data-testid="opener" onClick={() => setOpen(true)}>
90+
opener
91+
</button>
92+
{open && renderDialog(() => setOpen(false))}
93+
</>
94+
);
95+
}
96+
97+
describe.each(DIALOGS)('migrated dialog on <Modal>: $name', ({ accessibleName, render: renderDialog }) => {
98+
it('opens as an aria-modal dialog with the expected accessible name', async () => {
99+
const user = userEvent.setup();
100+
render(<Harness renderDialog={renderDialog} />);
101+
102+
await user.click(screen.getByTestId('opener'));
103+
104+
const dialog = screen.getByRole('dialog', { name: accessibleName });
105+
expect(dialog).toHaveAttribute('aria-modal', 'true');
106+
});
107+
108+
it('traps focus within the dialog when tabbing past the last element', async () => {
109+
const user = userEvent.setup();
110+
render(<Harness renderDialog={renderDialog} />);
111+
112+
await user.click(screen.getByTestId('opener'));
113+
const dialog = screen.getByRole('dialog');
114+
115+
const focusables = dialog.querySelectorAll(
116+
'button:not([disabled]), [href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])'
117+
);
118+
expect(focusables.length).toBeGreaterThan(0);
119+
120+
focusables[focusables.length - 1].focus();
121+
await user.tab();
122+
await waitFor(() => expect(dialog.contains(document.activeElement)).toBe(true));
123+
});
124+
125+
it('closes on Escape and returns focus to the opener', async () => {
126+
const user = userEvent.setup();
127+
render(<Harness renderDialog={renderDialog} />);
128+
129+
const opener = screen.getByTestId('opener');
130+
opener.focus();
131+
await user.click(opener);
132+
expect(screen.getByRole('dialog')).toBeInTheDocument();
133+
134+
await user.keyboard('{Escape}');
135+
await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
136+
await waitFor(() => expect(opener).toHaveFocus());
137+
});
138+
});
139+
140+
// Silence jsdom "not implemented" noise if any legacy path still reaches alert().
141+
vi.stubGlobal('alert', vi.fn());

‎src/__tests__/unit/a11y/contrast.test.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ const PAIRS = [
8989
['validation chip: valid', '#2e7d32', '#e8f5e9', 4.5],
9090
['validation chip: error', '#c62828', '#fdecea', 4.5],
9191
['validation chip: incomplete', '#bf360c', '#fff3e0', 4.5],
92+
// CalendarDayCreator muted/day-number text. The muted secondary text (weekday
93+
// headers, legend, close button) sits on the off-white legend panel (#fafafa);
94+
// adjacent-month day numbers sit on white and the grey hover background (#f5f5f5).
95+
// grey-600 keeps all of these >= 4.5:1 (off-white 5.50:1, hover 5.27:1).
96+
['calendar muted text on off-white legend', 'color-grey-600', '#fafafa', 4.5],
97+
['calendar other-month day number on hover grey', 'color-grey-600', '#f5f5f5', 4.5],
98+
// CopyFromAnimalDialog copy-preview text on its primary-light panel.
99+
['copy-preview text on primary-light panel', 'color-primary', 'color-primary-light', 4.5],
92100
];
93101

94102
describe('color contrast (audited workspace pairs meet AA)', () => {

‎src/__tests__/unit/state/workspace-animal.test.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,35 @@ describe('Animal State Management', () => {
419419
expect(animal.configurationHistory[1].version).toBe(2);
420420
expect(animal.configurationHistory[2].version).toBe(3);
421421
});
422+
423+
it('returns the created version number', () => {
424+
const { result } = renderHook(() => useStore());
425+
426+
act(() => {
427+
result.current.actions.createAnimal('remy', {
428+
species: 'Rattus norvegicus',
429+
sex: 'M',
430+
genotype: 'Wild Type',
431+
date_of_birth: '2023-01-10T00:00:00Z',
432+
description: 'Test subject',
433+
});
434+
});
435+
436+
// createAnimal seeds version 1, so the next snapshot is version 2.
437+
let returnedVersion;
438+
act(() => {
439+
returnedVersion = result.current.actions.addConfigurationSnapshot('remy', {
440+
date: '2023-06-15',
441+
description: 'Lowered tetrodes',
442+
devices: { electrode_groups: [], ntrode_electrode_group_channel_map: [] },
443+
});
444+
});
445+
446+
expect(returnedVersion).toBe(2);
447+
const animal = result.current.model.workspace.animals['remy'];
448+
expect(animal.configurationHistory).toHaveLength(2);
449+
expect(animal.configurationHistory[1].version).toBe(2);
450+
});
422451
});
423452

424453
describe('workspace.animals selector', () => {

0 commit comments

Comments
 (0)