feat(ui): D.6c rename local SectionHeader to NumberedSectionToggle - #78
Conversation
PR 10 (sub-step c) of 16. Last of three small PRs against
MeasurementDashboard.tsx — closes out the dashboard migration.
The local SectionHeader at :293 is structurally a collapsible toggle
button (the whole row IS the click target for open/close), not a
static h2 like the ui/SectionHeader primitive. Substituting the
primitive would either nest interactive elements or swallow the
collapsibility, so the local component stays — but it gets renamed
to NumberedSectionToggle so the distinction is obvious and the name
no longer shadows the primitive.
Also tightens the a11y of the toggle: adds aria-expanded={isOpen} on
the button and aria-hidden on the +/− glyph (purely decorative once
aria-expanded carries the state).
Preserved verbatim:
- data-text-role="meta" + data-text-role="section-title" attributes
(analysisResultsUi.test.ts spec-level vocabulary).
- All call sites — only the local component name changes; the props
and behavior are unchanged.
- All Section IDs / data-testid hooks (rhythm-grid-*, spectral-*).
Verified: lint clean, 628/628 unit tests passing.
D.6 (MeasurementDashboard) is now fully migrated across 10a/b/c.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
Renames the local SectionHeader component in MeasurementDashboard.tsx to NumberedSectionToggle and adds aria-expanded/aria-hidden a11y attributes. One file, 20 lines. The rename is complete — no orphaned call sites remain, and the only surviving SectionHeader references in the file are in the comment explaining the architectural distinction. The JSDoc comment correctly records why the local component cannot be replaced by the ui/SectionHeader primitive. Unit tests are reported 628/628 passing.
Findings
Worth considering:
NumberedSectionToggle now has aria-expanded but no aria-controls pointing at the controlled section div. Screen readers will announce the expanded/collapsed state but can't programmatically navigate to the revealed content. Not a regression (the previous code had neither), but since you're touching a11y anyway, wiring aria-controls to the section's data-testid id would complete the pattern.
Test results
628/628 unit passing per PR description. CI checks were in-progress at review time; the diff touches no logic — only a rename and two attribute additions — so a CI failure here would be environmental, not caused by this change.
Phase boundary check
Clean. No Phase 1, Phase 2, or Phase 3 code touched.
Generated by Claude Code
analysisResultsUi.test.ts:533 asserts ≥4 occurrences of the literal
class string `border-l-2 border-accent` in the rendered HTML. That
string was what AccentMetricCard emitted before the D.5a / D.6a
MetricTile migration; my MetricTile primitive used the directional
`border-l-{tone}` form (visually identical but a different literal),
so the test broke once D.6a removed the last AccentMetricCard.
Switch to `border-l-2 border-{tone}` — `border-l-2` carries the 2px
left thickness; the tone applies to all borders but only the left is
visible. Matches AccentMetricCard's class exactly and restores the
brittle assertion.
Same fix as the bundled patch in PR #79; cherry-pick to this branch
so #78 can complete its own CI gate independently.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR 14 of 16 in the UI consistency overhaul. Final cleanup pass. Removes three CSS utility classes from index.css: - .ableton-panel - .ableton-device - .ableton-header All three were retired by D.* migrations — every former consumer now uses the canonical primitives from components/ui/ (Panel, DeviceRack, SectionHeader). `grep -rn` against apps/ui/src/ confirms zero remaining usages. .ableton-shell and .ableton-toolbar STAY because App.tsx:952,956 still consumes them for the outer app chrome (the load-bearing section-grid shell). A focused follow-up could migrate those two to DeviceRack / a future AppShell primitive; out of scope here. Deferred (preserved as-is): - components/MeasurementPrimitives.tsx cannot be fully deleted yet — 5 consumers still depend on timeline/lane primitives (LaneContainer, LaneRow, StatsBar, TimeRuler) that the plan explicitly preserved. The shipped primitives (StatusBadge, DeltaBadge, MetricBar, MetricBarRow, OutlinePillButton, TokenBadgeList, StyledDataTable, AccentMetricCard) also still have consumers in MeasurementDashboard and AnalysisResults sections that weren't migrated (StructureLanes, HarmonyLanes, AnalysisResults remaining sections). Keep the file intact. - components/Tooltip.tsx stays as a re-export shim; the file isn't causing any drift and external imports may still target it. Verified: lint clean, 628/628 unit tests passing, build clean. D.* migrations summary (all merged or open at time of this PR): - D.0 Foundation (#64) ✓ - D.1 Primitives complete (#65) ✓ - D.2 SamplePlayback (#66) ✓ - D.3 SignalChain marquee (#67) ✓ - D.4 FileUpload + Input Source (#69) ✓ - D.5a Results header + metric strip (#71) ✓ - D.5b Mix Chain (#73) ✓ - D.5c Patches (#74) ✓ - D.5d Section headers (#75) ✓ - D.6a Dashboard structure (#76) ✓ - D.6b Dashboard sections (#77) ✓ - D.6c Dashboard headers (#78) ✓ - D.7 SessionMusicianPanel + MixDoctorPanel (#79) — open - D.8 Signal Monitor + SpectrogramViewer drift (#80) — open - D.9 DenseDawConcept drift (#81) — open - D.10 Cleanup (this PR) ✓ https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
…82) PR 14 of 16 in the UI consistency overhaul. Final cleanup pass. Removes three CSS utility classes from index.css: - .ableton-panel - .ableton-device - .ableton-header All three were retired by D.* migrations — every former consumer now uses the canonical primitives from components/ui/ (Panel, DeviceRack, SectionHeader). `grep -rn` against apps/ui/src/ confirms zero remaining usages. .ableton-shell and .ableton-toolbar STAY because App.tsx:952,956 still consumes them for the outer app chrome (the load-bearing section-grid shell). A focused follow-up could migrate those two to DeviceRack / a future AppShell primitive; out of scope here. Deferred (preserved as-is): - components/MeasurementPrimitives.tsx cannot be fully deleted yet — 5 consumers still depend on timeline/lane primitives (LaneContainer, LaneRow, StatsBar, TimeRuler) that the plan explicitly preserved. The shipped primitives (StatusBadge, DeltaBadge, MetricBar, MetricBarRow, OutlinePillButton, TokenBadgeList, StyledDataTable, AccentMetricCard) also still have consumers in MeasurementDashboard and AnalysisResults sections that weren't migrated (StructureLanes, HarmonyLanes, AnalysisResults remaining sections). Keep the file intact. - components/Tooltip.tsx stays as a re-export shim; the file isn't causing any drift and external imports may still target it. Verified: lint clean, 628/628 unit tests passing, build clean. D.* migrations summary (all merged or open at time of this PR): - D.0 Foundation (#64) ✓ - D.1 Primitives complete (#65) ✓ - D.2 SamplePlayback (#66) ✓ - D.3 SignalChain marquee (#67) ✓ - D.4 FileUpload + Input Source (#69) ✓ - D.5a Results header + metric strip (#71) ✓ - D.5b Mix Chain (#73) ✓ - D.5c Patches (#74) ✓ - D.5d Section headers (#75) ✓ - D.6a Dashboard structure (#76) ✓ - D.6b Dashboard sections (#77) ✓ - D.6c Dashboard headers (#78) ✓ - D.7 SessionMusicianPanel + MixDoctorPanel (#79) — open - D.8 Signal Monitor + SpectrogramViewer drift (#80) — open - D.9 DenseDawConcept drift (#81) — open - D.10 Cleanup (this PR) ✓ https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe Co-authored-by: Claude <noreply@anthropic.com>
Summary
PR 10 (sub-step c) of 16. Last of three small PRs against
MeasurementDashboard.tsx— closes out the dashboard migration.The local
SectionHeaderat:293is structurally a collapsible toggle button (the whole row IS the click target for open/close), not a static<h2>like theui/SectionHeaderprimitive. Substituting the primitive would either nest interactive elements or swallow the collapsibility, so the local component stays — but it gets renamed toNumberedSectionToggleso the distinction is obvious and the name no longer shadows the primitive.Also tightens the a11y of the toggle: adds
aria-expanded={isOpen}on the button andaria-hiddenon the+/−glyph (purely decorative oncearia-expandedcarries the state).Preserved verbatim
data-text-role="meta"+data-text-role="section-title"attributes (analysisResultsUi.test.tsspec-level vocabulary).data-testidhooks (rhythm-grid-*,spectral-*).Test plan
npm run lint— cleannpm run test:unit— 628/628 passingnpm run test:smoke— sandbox blocks Playwright browser; data-testids + text + ARIA preservedStatus
D.6 (MeasurementDashboard) is now fully migrated across 10a / 10b / 10c.
Up next
D.7 — SessionMusicianPanel + MixDoctorPanel.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Generated by Claude Code