feat(ui): D.5d migrate ResultsSectionHeader to SectionHeader primitive - #75
Conversation
PR 9 of 16 in the UI consistency overhaul. Promotes every section header in AnalysisResults to the SectionHeader primitive via a single internal change — the local ResultsSectionHeader helper is now a thin wrapper that delegates to <SectionHeader variant="underline">. Affected sections (every ResultsSectionHeader call site): - Interpretation panel (:1024) - Idle / "waiting on interpretation" panel (:1214) - Style Profile (:1227) - Audio Observations (:1369) - Project Setup (:1436) - Track Layout (:1466) - Routing Blueprint (:1529) - Warp Guide (:1601) - (second warp section :1643) - Arrangement (:1689) - Sonic Elements & Reconstruction (:1891) - Mix & Master Chain (already migrated in D.5b but still uses the helper) - Patch Framework (already migrated in D.5c but still uses the helper) What changes visually: - The static accent dot in each section header (<span class="w-2 h-2 bg-accent rounded-full">) becomes the pulsing `.led-indicator--active` glyph. This is the same LED affordance used on every DeviceRack title strip in PRs #66-#74, so all section landmarks now share one vocabulary. - The outer underline + flex layout is preserved verbatim (border-b border-border pb-2 + flex items-center justify-between gap-3), so spacing doesn't shift. What's preserved verbatim: - The ResultsSectionHeader prop API (title, rightSlot, titleRole, titleClassName, className) — every existing call site continues to work without changes. - All <h2> elements with data-text-role={titleRole} — analysisResultsUi.test.ts:465-466 assertions (data-text-role="section-title" + ">Mix & Master Chain</h2>) continue to match. - data-text-role="item-title" assertions inside each section. - All data-testid hooks (interpretation-panel, interpretation- warnings, mix-chain-applied-progress, patches-applied-progress, measurements-section, etc.). - The `border-b border-border pb-2` outer container class so spacing is identical. Deferred (not in D.5d scope): - Per-section DeviceRack wraps were considered (per the plan's "Section wrappers → <DeviceRack>" item) but would have created duplicate title strips wherever a section also has a ResultsSectionHeader — the rack title strip + the SectionHeader h2 would both render section identity. Each affected section would need a custom "drop the SectionHeader, rely only on the rack name" call which doesn't fit a single mechanical change. Save for a focused follow-up if/when the rack chrome is wanted on these surfaces. Verified: lint clean, 628/628 unit tests passing, build clean (AnalysisResults chunk 254 KB / 63 KB gz — marginally smaller than PR 8 due to less inline JSX in the section header). https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (posted as COMMENT — can't self-approve)
Summary
Single-file UI migration. ResultsSectionHeader becomes a thin delegate to <SectionHeader variant="underline" size="md" ledTone="accent">, replacing the inline static-dot implementation. The prop API (title, rightSlot→action, titleRole, titleClassName, className) maps cleanly. SectionHeader renders <h2 data-text-role={titleRole}> (primitive line 88-89), so the analysisResultsUi.test.ts:465-466 assertions hold. variant="underline" maps directly to border-b border-border pb-2 in sectionHeaderVariants. Class computation equivalence confirmed: textRoleClassName(role, extras) (line 312-313) is exactly getTextRoleClassName(role) + ' ' + extras, which is what SectionHeader reconstructs via cn(titleBase, 'flex items-center gap-2', titleClassName). Vitest was not runnable in this review environment, but code inspection confirms all load-bearing assertions survive.
Findings
Worth considering (non-blocking × 2):
-
SectionHeaderwraps the title in an inner<div className="flex flex-col gap-0.5 min-w-0">(primitive line 79) not present in the old implementation.min-w-0prevents title overflow in flex rows that have arightSlot— strictly better behavior.gap-0.5is a no-op becauseeyebrowis never passed byResultsSectionHeader. The PR description doesn't mention this structural addition; worth a line in the migration plan doc if you're tracking visual-layout surface area. -
The
actioncontainer inSectionHeader(primitive line 96) carriesgap-2that the oldrightSlotwrapper lacked. All currentrightSlotcall sites pass a single<span>, so no visible difference today. Only relevant if a future caller passes multiple children.
Test results
Vitest not installed in review environment; could not run locally. PR claims 628/628. Code inspection confirms the data-text-role, <h2> text, and data-testid assertions are intact.
Phase boundary check
Clean. No Phase 1 schema touches, no Phase 2/3 measurement reads or writes. Pure UI primitive wiring.
Generated by Claude Code
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 9 of 16 in the UI consistency overhaul / Live 12 restyle stack. Promotes every section header in
AnalysisResultsto theSectionHeaderprimitive via a single internal change — the localResultsSectionHeaderhelper is now a thin wrapper that delegates to<SectionHeader variant="underline">.Affected sections (every
ResultsSectionHeadercall site)What changes visually
<span class="w-2 h-2 bg-accent rounded-full">) becomes the pulsing.led-indicator--activeglyph. Same affordance as every DeviceRack title strip in PRs feat(ui): D.2 migrate SamplePlayback to ui/ primitives #66-feat(ui): D.5c migrate Patches per-group sections to DeviceRack #74, so all section landmarks now share one vocabulary.border-b border-border pb-2+flex items-center justify-between gap-3), so spacing doesn't shift.What's preserved verbatim
ResultsSectionHeaderprop API (title,rightSlot,titleRole,titleClassName,className) — every existing call site continues to work without changes.<h2>elements withdata-text-role={titleRole}—analysisResultsUi.test.ts:465-466assertions (data-text-role="section-title"+>Mix & Master Chain</h2>) continue to match.data-text-role="item-title"assertions inside each section.data-testidhooks (interpretation-panel,interpretation-warnings,mix-chain-applied-progress,patches-applied-progress,measurements-section, etc.).border-b border-border pb-2outer container class so spacing is identical.Deferred (not in D.5d scope)
Per-section
DeviceRackwraps were considered (per the plan's "Section wrappers →<DeviceRack>" item) but would have created duplicate title strips wherever a section also has aResultsSectionHeader— the rack title strip + theSectionHeaderh2 would both render section identity. Each affected section would need a custom "drop the SectionHeader, rely only on the rack name" call which doesn't fit a single mechanical change. Save for a focused follow-up if/when the rack chrome is wanted on these surfaces.Test plan
npm run lint— cleannpm run test:unit— 628/628 passing (includinganalysisResultsUi.test.tsthat assertsdata-text-role+ theMix & Master Chainh2 text)npm run build— clean (AnalysisResults chunk 254 KB / 63 KB gz, marginally smaller than PR 8)npm run test:smoke— sandbox blocks Playwright browser; alldata-testid+ ARIA + heading text preservedUp next (PR 10a)
D.6a MeasurementDashboard structure — wrap the dashboard in
<DeviceRack name="MEASUREMENTS" density="dense">. MigrateAccentMetricCardusages →<MetricTile>(API parity, drop-in). Preservedata-testid="measurement-dashboard".https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Generated by Claude Code