Skip to content

Commit 094c805

Browse files
edenoclaude
andcommitted
feat: make the workspace UI discoverable, escapable, and honest (a11y)
Navigation/discoverability, stub honesty, and accessibility hardening for the new multi-page UI. The default route stays the legacy form (cutover is a later phase); no YAML path touched, golden baselines unchanged. - Add a persistent primary nav (Home/Workspace, aria-current, flag-gated legacy toggle) in AppLayout, rendered on non-legacy routes; document the routing contract (new routes reachable regardless of flags; cutover flips flags + default later). - Wire the Devices step status to real validation (computeDevicesStatus mirrors DevicesStep); isExportEnabled unchanged. Epochs/Validation remain not-yet-built. - Make stub steps honest: Epochs/Validation/Export are visibly unavailable with neutral "coming soon" copy. - Remove dead-ends: error screens get Workspace + Home escapes and a main landmark; AnimalEditor gains a back-to-workspace link; fix OverviewStep #/animal links to /editor and the breadcrumb Home to #/home. - Fix the Home post-create navigation race (navigate from confirmed state, no setTimeout) and the AnimalWorkspace batch-day dedupe (local id set, no stale snapshot). - Accessibility: single banner/contentinfo + single main per route (stepper header/footer are plain divs); global .sr-only/.visually-hidden; stub steps use aria-disabled (discoverable by screen readers) with meaningful locked labels; 44px touch targets; AnimalEditor step status conveyed beyond color; consistent back-link labels; ElectrodeGroupModal gains an interim focus trap + focus-return (to be replaced by the shared Modal primitive in a later phase). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1bb01f7 commit 094c805

26 files changed

Lines changed: 717 additions & 116 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–1 complete (merged to `modern`). Phase 2 next.
3+
**Status:** Phases 0–2 complete (merged to `modern`). Phase 3 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

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

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
* @vitest-environment jsdom
33
*/
44

5-
import { describe, it, expect } from 'vitest';
6-
import { render } from '@testing-library/react';
5+
import { describe, it, expect, afterEach } from 'vitest';
6+
import { render, screen, waitFor } from '@testing-library/react';
77
import { StoreProvider } from '../../state/StoreContext';
88
import { App } from '../../App';
9+
import { overrideFlags, restoreFlags } from '../../featureFlags';
10+
import { makeTestWorkspace } from '../helpers/test-fixtures';
911

1012
/**
1113
* Integration tests for ARIA landmarks (P1.1.4)
@@ -144,4 +146,90 @@ describe('ARIA Landmarks', () => {
144146
});
145147
});
146148
});
149+
150+
describe('Per-route landmarks (new workspace UI)', () => {
151+
const renderRoute = (hash) => {
152+
window.location.hash = hash;
153+
return render(
154+
<StoreProvider initialState={{ workspace: makeTestWorkspace() }}>
155+
<App />
156+
</StoreProvider>
157+
);
158+
};
159+
160+
afterEach(() => {
161+
restoreFlags();
162+
window.location.hash = '';
163+
});
164+
165+
// makeTestWorkspace seeds animal "remy" + day "remy_20230622".
166+
it('Home: one main + one #main-content, a navigation landmark, and a Workspace link', () => {
167+
overrideFlags({ animalWorkspace: true });
168+
const { container } = renderRoute('#/home');
169+
170+
expect(container.querySelectorAll('[role="main"]')).toHaveLength(1);
171+
expect(container.querySelectorAll('#main-content')).toHaveLength(1);
172+
expect(container.querySelector('[role="navigation"]')).toBeTruthy();
173+
// Escape to the workspace exists via the primary nav.
174+
expect(screen.getByRole('link', { name: /^workspace$/i })).toBeInTheDocument();
175+
});
176+
177+
it('Workspace: one main + one #main-content, a navigation landmark, escape to Home', () => {
178+
overrideFlags({ animalWorkspace: true });
179+
const { container } = renderRoute('#/workspace');
180+
181+
expect(container.querySelectorAll('[role="main"]')).toHaveLength(1);
182+
expect(container.querySelectorAll('#main-content')).toHaveLength(1);
183+
expect(container.querySelector('[role="navigation"]')).toBeTruthy();
184+
expect(screen.getByRole('link', { name: /^home$/i })).toBeInTheDocument();
185+
});
186+
187+
it('DayEditor: one main + one #main-content + one banner/contentinfo; back-to-workspace link', async () => {
188+
overrideFlags({ animalWorkspace: true, newDayEditor: true });
189+
const { container } = renderRoute('#/day/remy_20230622');
190+
191+
// useDayIdFromUrl resolves the day id in an effect, so wait for the real stepper.
192+
const back = await screen.findByRole('link', { name: /back to workspace/i });
193+
expect(back).toBeInTheDocument();
194+
expect(container.querySelectorAll('[role="main"]')).toHaveLength(1);
195+
expect(container.querySelectorAll('#main-content')).toHaveLength(1);
196+
// The stepper header/footer are plain divs, so AppLayout owns the only banner
197+
// and contentinfo landmarks (no duplicates).
198+
expect(container.querySelectorAll('[role="banner"], header')).toHaveLength(1);
199+
expect(container.querySelectorAll('[role="contentinfo"], footer')).toHaveLength(1);
200+
});
201+
202+
it('AnimalEditor: exactly one main + one #main-content (duplicate removed); back-to-workspace link', async () => {
203+
overrideFlags({ animalWorkspace: true });
204+
const { container } = renderRoute('#/animal/remy/editor');
205+
206+
// AnimalEditor is lazy-loaded behind Suspense.
207+
await waitFor(() => {
208+
expect(container.querySelectorAll('[role="main"]')).toHaveLength(1);
209+
});
210+
expect(container.querySelectorAll('#main-content')).toHaveLength(1);
211+
// Stepper header/footer are plain divs → AppLayout owns the only banner/contentinfo.
212+
expect(container.querySelectorAll('[role="banner"], header')).toHaveLength(1);
213+
expect(container.querySelectorAll('[role="contentinfo"], footer')).toHaveLength(1);
214+
expect(screen.getByRole('link', { name: /back to workspace/i })).toBeInTheDocument();
215+
});
216+
217+
it('keeps the default route (#/) on the legacy form even with flags enabled', () => {
218+
overrideFlags({ animalWorkspace: true, newDayEditor: true });
219+
window.location.hash = '#/';
220+
// No seeded workspace here: the legacy form reads the flat formData model, so
221+
// seeding only { workspace } would leave formData without its fields.
222+
const { container } = render(
223+
<StoreProvider>
224+
<App />
225+
</StoreProvider>
226+
);
227+
228+
// Default stays legacy this phase (cutover flips the default in a later phase).
229+
expect(container.querySelector('form')).toBeTruthy();
230+
// Legacy supplies a single main; the primary nav is not rendered on legacy.
231+
expect(container.querySelectorAll('[role="main"]')).toHaveLength(1);
232+
expect(screen.queryByRole('navigation', { name: /primary/i })).not.toBeInTheDocument();
233+
});
234+
});
147235
});

‎src/index.css‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,62 @@ body {
22
font-family: sans-serif;
33
}
44

5+
/* Global screen-reader-only utility. Defined here (index.css is loaded on every
6+
route) so the always-present route announcer and per-component sr-only labels are
7+
visually hidden on the new workspace routes too, not just where a route-specific
8+
stylesheet happens to define it. */
9+
.sr-only,
10+
.visually-hidden {
11+
position: absolute !important;
12+
width: 1px;
13+
height: 1px;
14+
padding: 0;
15+
margin: -1px;
16+
overflow: hidden;
17+
clip: rect(0, 0, 0, 0);
18+
white-space: nowrap;
19+
border: 0;
20+
}
21+
22+
/* Primary navigation into the workspace (rendered on non-legacy routes). Sits in
23+
normal flow below the banner; positioned/stacked so banner content can't overlap
24+
and intercept clicks on the links. */
25+
.primary-nav {
26+
position: relative;
27+
z-index: 1;
28+
display: flex;
29+
flex-wrap: wrap;
30+
gap: 0.5rem;
31+
align-items: center;
32+
padding: 0.5rem 10px;
33+
border-bottom: 1px solid #e0e0e0;
34+
background: #fafafa;
35+
}
36+
37+
.primary-nav a {
38+
display: inline-flex;
39+
align-items: center;
40+
min-height: 44px;
41+
padding: 0.4rem 0.9rem;
42+
border-radius: 4px;
43+
color: #1565c0;
44+
text-decoration: none;
45+
}
46+
47+
.primary-nav a:hover {
48+
background: #eef3fb;
49+
text-decoration: underline;
50+
}
51+
52+
.primary-nav a[aria-current='page'] {
53+
font-weight: 600;
54+
background: #e3f0ff;
55+
}
56+
57+
.primary-nav .legacy-toggle {
58+
margin-left: auto;
59+
}
60+
561
/* Notice shown when a previously-saved workspace could not be restored. */
662
.load-notice {
763
display: flex;

‎src/layouts/AppLayout.jsx‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import React, { useEffect, useRef, lazy, Suspense } from 'react';
1111
import { useHashRouter } from '../hooks/useHashRouter';
12+
import { isFeatureEnabled } from '../featureFlags';
1213
import { useStoreContext } from '../state/StoreContext';
1314
import { useUnsavedWorkGuard } from '../hooks/useUnsavedWorkGuard';
1415
import { Home } from '../pages/Home';
@@ -115,7 +116,17 @@ export function AppLayout() {
115116
}, [currentRoute]);
116117

117118
/**
118-
* Render current view based on route
119+
* Render current view based on route.
120+
*
121+
* Routing contract (see shared-contracts "Feature flags & routing"): the default
122+
* route (`#/`) renders the legacy form, and the new workspace routes (`#/home`,
123+
* `#/workspace`, `#/day/:id`, `#/animal/:id/editor`, `#/validation`) render their
124+
* views and remain reachable for development regardless of the feature flags.
125+
* The cutover (a single switch in a later phase) flips `animalWorkspace` /
126+
* `newDayEditor` on and changes the *default* route to the workspace; nothing else
127+
* in this resolution changes. We intentionally do NOT gate the explicit new routes
128+
* behind the flags here, so they stay reachable while the default stays legacy.
129+
*
119130
* @returns {React.Element} Current view component
120131
*/
121132
function renderView() {
@@ -179,6 +190,35 @@ export function AppLayout() {
179190
</a>
180191
</div>
181192

193+
{/*
194+
Primary navigation makes the workspace discoverable. Rendered on the new
195+
(non-legacy) routes only: the legacy view supplies its own in-page
196+
"Form section navigation" landmark, so scoping this here keeps exactly one
197+
navigation landmark per route. The "Use Legacy Editor" toggle is hidden until
198+
the cutover enables `showLegacyToggle`.
199+
*/}
200+
{currentRoute.view !== 'legacy' && (
201+
<nav className="primary-nav" role="navigation" aria-label="Primary">
202+
<a
203+
href="#/home"
204+
aria-current={currentRoute.view === 'home' ? 'page' : undefined}
205+
>
206+
Home
207+
</a>
208+
<a
209+
href="#/workspace"
210+
aria-current={currentRoute.view === 'workspace' ? 'page' : undefined}
211+
>
212+
Workspace
213+
</a>
214+
{isFeatureEnabled('showLegacyToggle') && (
215+
<a href="#/" className="legacy-toggle">
216+
Use Legacy Editor
217+
</a>
218+
)}
219+
</nav>
220+
)}
221+
182222
{/* Notice when previously-saved workspace data could not be restored, so a
183223
discarded (corrupt / incompatible-version) workspace is never silent. */}
184224
{persistence.loadNotice && (

‎src/layouts/__tests__/AppLayout.test.jsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,37 @@ describe('AppLayout', () => {
441441
});
442442
});
443443

444+
describe('primary navigation', () => {
445+
it('is hidden on the legacy route (legacy supplies its own nav)', () => {
446+
window.location.hash = '#/';
447+
render(<AppLayout />);
448+
expect(screen.queryByRole('navigation', { name: /primary/i })).not.toBeInTheDocument();
449+
});
450+
451+
it('renders Home and Workspace links on non-legacy routes', () => {
452+
window.location.hash = '#/home';
453+
render(<AppLayout />);
454+
455+
const nav = screen.getByRole('navigation', { name: /primary/i });
456+
expect(nav).toBeInTheDocument();
457+
expect(screen.getByRole('link', { name: /^home$/i })).toHaveAttribute('href', '#/home');
458+
expect(screen.getByRole('link', { name: /^workspace$/i })).toHaveAttribute('href', '#/workspace');
459+
});
460+
461+
it('marks the current route link with aria-current=page', () => {
462+
window.location.hash = '#/workspace';
463+
render(<AppLayout />);
464+
expect(screen.getByRole('link', { name: /^workspace$/i })).toHaveAttribute('aria-current', 'page');
465+
expect(screen.getByRole('link', { name: /^home$/i })).not.toHaveAttribute('aria-current');
466+
});
467+
468+
it('hides the "Use Legacy Editor" toggle while showLegacyToggle is off', () => {
469+
window.location.hash = '#/home';
470+
render(<AppLayout />);
471+
expect(screen.queryByRole('link', { name: /use legacy editor/i })).not.toBeInTheDocument();
472+
});
473+
});
474+
444475
describe('discarded-workspace notice', () => {
445476
afterEach(() => {
446477
window.localStorage.clear();

0 commit comments

Comments
 (0)