feat(ui): D.5c migrate Patches per-group sections to DeviceRack - #74
Conversation
PR 8 of 16 in the UI consistency overhaul. Patches section follows the Mix Chain (D.5b / PR #73) shape — each processing-stage group becomes a <DeviceRack>; the progress badge becomes a <Pill>. Branched off main. Single-file PR — AnalysisResults.tsx. Migration: - Per-group <section> + <h3> heading → <DeviceRack name={emoji+group.name}>. - patches-applied-progress: hand-rolled span → <Pill tone="success" size="sm">. data-testid preserved. Also includes a bug fix carried over from D.5b: the `name` prop is now a JSX fragment instead of a template literal. groupIcon('BASS PROCESSING') returns an <AudioWaveform> SVG node; template-literal interpolation stringified it to "[object Object]" — visually broken in production AND a hidden test regression that PR #73 narrowly avoided because the Patches section still rendered the raw <h3> with the SVG, satisfying analysisResultsUi.test.ts:448's `/lucide-audio-waveform[\s\S]{0,200}BASS PROCESSING/` regex via Patches alone. Migrating Patches in this PR would have removed the last unbroken match site and failed the test — so both Mix Chain and Patches now use the JSX-fragment form and the SVG renders in both groups as intended. Deferred (same scope as D.5b): - Per-card <div> outer wrapper NOT migrated to <Panel>. The card has rich state-dependent styling (bg-bg-card hover, conditional border-l-2 for applied, overflow-hidden) that would need Panel extensions. - AppliedCheckbox NOT migrated to Radix Checkbox primitive (would change green→orange palette; existing button has equivalent a11y). Preserved verbatim: - data-testids: patches-applied-progress, applied-checkbox, patch-headline-*, citation-row-*. - Visible text: group names + emojis/SVG, "applied", "Sources", Transcription-derived, etc. - The exact `grid gap-4 grid-cols-1 sm:grid-cols-2` className (now appears in both Mix Chain and Patches racks, still ≥2 occurrences per analysisResultsUi.test.ts:440). - All data-text-role attributes. - CitationBlock + CitationHeadline internals. Verified: lint clean, 628/628 unit tests passing (including analysisResultsUi.test.ts which now correctly asserts both the grid className AND the lucide-audio-waveform SVG class for BASS PROCESSING), build clean (AnalysisResults chunk 254 KB / 63 KB gz). https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
#75) 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 Co-authored-by: Claude <noreply@anthropic.com>
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
Pure UI refactor on one file. Patches per-group sections migrate from <section> + <h3> to DeviceRack, matching the D.5b Mix Chain shape. Also retroactively fixes D.5b's template-literal bug for Mix Chain groups (where groupIcon() returns an SVG node and template-literal coercion stringified it to "[object Object]"). The JSX fragment fix is correct. data-testids, visible text, and the grid gap-4 grid-cols-1 sm:grid-cols-2 className that the test suite counts are all preserved.
Findings
No blocking or should-fix findings.
Worth considering: the <Pill tone="success" size="sm"> swap changes the visual styling from the hand-rolled text-[10px] font-mono uppercase tracking-wide text-success border border-success/30 bg-success/10 px-2 py-1 rounded span. Functionally identical for the test suite (testid preserved), but if the Pill component's success tone renders a meaningfully different palette or size, the manual verification step in the test plan is where that shows up. Not a code issue — just noting it's a visual delta the description doesn't enumerate.
Test results
628 / 628 pass. 49 test files. Lint (tsc --noEmit) clean. Verified locally on the PR HEAD (73d9428).
Phase boundary check
Clean. No Phase 1, Phase 2, or Phase 3 measurement logic touched.
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 8 of 16 in the UI consistency overhaul / Live 12 restyle stack. Patches section follows the Mix Chain (D.5b / #73) shape — each processing-stage group becomes a
<DeviceRack>; the progress badge becomes a<Pill>.Migration
<section>+<h3>heading →<DeviceRack name={emoji+group.name}>.patches-applied-progress: hand-rolled<span>→<Pill tone="success" size="sm">.data-testidpreserved.Bug fix bundled — JSX fragment for the
namepropCarried over from D.5b: the
nameprop is now a JSX fragment instead of a template literal.groupIcon('BASS PROCESSING')returns an<AudioWaveform>SVG node; template-literal interpolation stringifies it to"[object Object]"— visually broken in production AND a hidden test regression that #73 narrowly avoided because the Patches section still rendered the raw<h3>with the SVG, satisfyinganalysisResultsUi.test.ts:448's/lucide-audio-waveform[\s\S]{0,200}BASS PROCESSING/regex via Patches alone. Migrating Patches in this PR would have removed the last unbroken match site and failed the test — so both Mix Chain and Patches now use the JSX-fragment form and the SVG renders in both groups as intended.Deferred (same scope as D.5b)
<div>outer wrapper NOT migrated to<Panel>. The card has rich state-dependent styling (bg-bg-cardhover, conditionalborder-l-2for applied,overflow-hidden) that would need Panel extensions.AppliedCheckboxNOT migrated to Radix Checkbox primitive (would change green→orange palette; existing button has equivalent a11y).Preserved verbatim
data-testids:patches-applied-progress,applied-checkbox,patch-headline-*,citation-row-*.grid gap-4 grid-cols-1 sm:grid-cols-2className (now appears in both Mix Chain and Patches racks; ≥2 occurrences peranalysisResultsUi.test.ts:440).data-text-roleattributes.CitationBlock+CitationHeadlineinternals.Test plan
npm run lint— cleannpm run test:unit— 628/628 passing (includinganalysisResultsUi.test.tswhich now correctly asserts both the grid className AND thelucide-audio-waveformSVG class for BASS PROCESSING)npm run build— clean (AnalysisResults chunk 254 KB / 63 KB gz)npm run test:smoke— sandbox blocks Playwright browser; selectors on text +data-testidpreservedUp next (PR 9)
D.5d Sonic Elements + remaining sections. Section headers →
<SectionHeader variant="underline">. Section wrappers →<DeviceRack>. Preservedata-testid="measurements-section"and similar.https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Generated by Claude Code