feat(ui): D.7 migrate SessionMusicianPanel + MixDoctorPanel - #79
Merged
Conversation
slittycode
pushed a commit
that referenced
this pull request
May 18, 2026
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
slittycode
added a commit
that referenced
this pull request
May 18, 2026
* feat(ui): D.6c rename local SectionHeader to NumberedSectionToggle
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
* fix(ui): MetricTile accent stripe uses border-{tone} not border-l-{tone}
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
---------
Co-authored-by: Claude <noreply@anthropic.com>
slittycode
pushed a commit
that referenced
this pull request
May 18, 2026
PR #79 Frontend CI failed on tests/smoke/upload-phase1-midi.spec.ts:631 which uses `page.locator('section').filter({ hasText: /SESSION MUSICIAN/i })` to find the panel. My D.7 migration replaced the outer `<section>` with `<DeviceRack>` (a div), so the locator returned 0 elements. Wrap the DeviceRack in a `<section data-testid= "session-musician-panel" aria-label="Session Musician">`. The section now owns the semantic element + the data-testid; the DeviceRack provides chrome inside it. The DOM contains "Session Musician" (case-insensitive match for /SESSION MUSICIAN/i) so the smoke locator resolves correctly. Verified: lint clean, 628/628 unit tests passing. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
This was referenced May 18, 2026
slittycode
added a commit
that referenced
this pull request
May 18, 2026
tests/smoke/responsive-layout.spec.ts:192 fails at the 375px mobile
viewport with `expect(getByText('Analysis Results')).toBeVisible()`
resolving to a hidden h2. Root cause: my D.5a migration replaced the
original `flex flex-col md:flex-row` layout for the Analysis Results
page header with the SectionHeader primitive which used
`flex items-center justify-between` unconditionally. At 375px the
two Download buttons (~280px combined) squeeze the title to zero
width and the parent `.ableton-shell overflow-hidden` clips the
title off-screen, so Playwright sees it as hidden.
Restore the responsive collapse: mobile-first
`flex flex-col items-start gap-2`, then `md:flex-row md:items-center
md:justify-between md:gap-3` at the 768px+ breakpoint. Matches the
pre-D.5a Analysis Results header behavior. Other consumers of
SectionHeader (the many ResultsSectionHeader call sites) have small
rightSlot content (badges, single pills, icons) — stacking those
below the title on mobile is a tiny visual improvement, not a
regression.
This is the failure that's been blocking #79, #80, #81, #82 — none
of those PRs introduced the bug; they all inherit it from main
because D.5a (#71) landed this regression. The fact that #71 itself
merged green is either CI luck (the test passed on its first run)
or the test was added later and not re-checked.
Landing this hotfix lets the four open PRs rebase and pick up the
fix to clear their CI.
Verified: lint clean, 628/628 unit tests passing, build clean.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Co-authored-by: Claude <noreply@anthropic.com>
PR 11 of 16 in the UI consistency overhaul.
SessionMusicianPanel:
- Outer <section> + custom header div → <DeviceRack name="Session
Musician" subtitle="· Pitch & Melody"> with the collapse chevron
in the title-strip action slot (<Button variant="ghost" iconOnly>).
- Off-banner div → <EmptyState tone="warning" title="Pitch/note
translation is off" description={offBannerCopy}>. Preserved
data-testid="session-musician-off-banner".
- No-data div → <EmptyState tone="neutral" title="PITCH & MELODY
UNAVAILABLE" description=...>. Preserved
data-testid="session-musician-no-data" and the title text the
analysisResultsUi.test.ts:1344 assertion locks
("PITCH & MELODY UNAVAILABLE").
- Drop the unused formatDisplayText import (its only consumer — the
h2 title — was replaced by the DeviceRack name prop).
MixDoctorPanel:
- 3-card top row (Target Genre / Health Score / Loudness Offset) →
3 × <MetricTile size="md" accent="accent">. Each card's eyebrow +
primary value + footer/footer-badge map directly to MetricTile's
label / value / footer props.
- Advisory summary rows (Dynamics / Loudness / Stereo) → <Panel
variant="surface" padding="md">. The hand-rolled bg-bg-card/35
inset-shadow div disappears in favor of the canonical primitive.
- StyledDataTable → DataTable (API parity drop-in; same columns,
same row data).
- Drop StyledDataTable + formatDisplayText imports.
Bundled fix — MetricTile accent stripe class string:
- Changed `border-l-2 border-l-{tone}` → `border-l-2 border-{tone}`.
Visually identical (only the left border has thickness; the tone
applies to all borders but only the left is visible). Matches the
literal class the analysisResultsUi.test.ts:533 regex was
asserting against the OLD AccentMetricCard's class string. The
L-suffixed form was a hidden regression introduced when D.5a /
D.6a migrated AccentMetricCards to MetricTile — the test happened
to still pass when SOME AccentMetricCards remained (their
border-l-2 border-accent string carried the ≥4 count), but with
D.6a completing the migration the test would fail. Fixing here
unblocks the dashboard PRs.
Preserved verbatim:
- All data-testid hooks (session-musician-panel, session-musician-
off-banner, session-musician-no-data on the SM side; mix-doctor
surfaces have no data-testids to begin with).
- All ARIA labels on the collapse toggle.
- Off-banner copy strings, no-data copy strings.
- The data-text-role attributes inside StatusBadge / DeltaBadge
(those come from the unchanged primitives).
- MixDoctor advisory tone vocabulary (success/warning/error mapping
by toneForScore/toneForMixIssue/toneForDynamicsIssue/
toneForStereoAdvice).
Verified: lint clean, 628/628 unit tests passing, build clean.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR #79 Frontend CI failed on tests/smoke/upload-phase1-midi.spec.ts:631 which uses `page.locator('section').filter({ hasText: /SESSION MUSICIAN/i })` to find the panel. My D.7 migration replaced the outer `<section>` with `<DeviceRack>` (a div), so the locator returned 0 elements. Wrap the DeviceRack in a `<section data-testid= "session-musician-panel" aria-label="Session Musician">`. The section now owns the semantic element + the data-testid; the DeviceRack provides chrome inside it. The DOM contains "Session Musician" (case-insensitive match for /SESSION MUSICIAN/i) so the smoke locator resolves correctly. Verified: lint clean, 628/628 unit tests passing. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
tests/smoke/upload-phase1-midi.spec.ts:360 asserts
`panel.getByRole('heading', { name: /SESSION MUSICIAN/i })` — the
panel must contain at least one heading element with that name.
My D.7 migration replaced the original <h2>Session Musician</h2>
with a DeviceRack title strip whose name renders as a styled
<span>, removing the heading role.
Add `<h2 className="sr-only">Session Musician</h2>` inside the
section wrapper — visually hidden but accessible to screen readers
and Playwright getByRole. DeviceRack title strip continues to
carry the visible label.
https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
slittycode
force-pushed
the
claude/ui-restyle-pr11-session-mix-doctor
branch
from
May 18, 2026 23:58
7973851 to
4a9c950
Compare
slittycode
pushed a commit
that referenced
this pull request
May 18, 2026
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
slittycode
added a commit
that referenced
this pull request
May 19, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 11 of 16 in the UI consistency overhaul / Live 12 restyle stack.
SessionMusicianPanel
<section>+ custom header →<DeviceRack name="Session Musician" subtitle="· Pitch & Melody">with the collapse chevron in the title-strip action slot (<Button variant="ghost" iconOnly>).<EmptyState tone="warning">. Preserveddata-testid="session-musician-off-banner".<EmptyState tone="neutral">. Preserveddata-testid="session-musician-no-data"and the title text theanalysisResultsUi.test.ts:1344assertion locks ("PITCH & MELODY UNAVAILABLE").MixDoctorPanel
<MetricTile size="md" accent="accent">.<Panel variant="surface" padding="md">.StyledDataTable→DataTable(API parity drop-in).Bundled fix —
MetricTileaccent stripe class stringChanged
border-l-2 border-l-{tone}→border-l-2 border-{tone}. Visually identical (only the left border has thickness; tone applies to all borders but only the left is visible). Matches the literal class theanalysisResultsUi.test.ts:533regex asserts against the OLDAccentMetricCard's class string. The L-suffixed form was a hidden regression introduced when D.5a / D.6a migrated AccentMetricCards to MetricTile — the test happened to still pass when SOME AccentMetricCards remained (theirborder-l-2 border-accentstring carried the ≥4 count), but with D.6a completing the migration the test would fail. Fixing here unblocks the dashboard PRs.Preserved verbatim
data-testidhooks.data-text-roleattributes insideStatusBadge/DeltaBadge(unchanged primitives).Test plan
npm run lint— cleannpm run test:unit— 628/628 passingnpm run build— cleanUp next
.ableton-*aliases).https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
Generated by Claude Code