11 KiB
Design Brief: Player Minimize/Spacer Sync
Status: Proposed — awaiting direction
Surface: DeepDrftPublic.Client audio player dock + MainLayout spacer
Author: product-designer
1. Root cause
AudioPlayerBar owns _isMinimized as private component state. MainLayout renders a
sibling spacer <div class="player-spacer @_audioPlayerClass"> whose class
(minimized / expanded) reserves vertical space for the fixed dock so page content
does not overlap it. The two are kept in sync by a single bridge: the
EventCallback<bool> OnMinimized parameter.
_isMinimized is mutated on three paths. Only one of them fires the bridge:
| Path | Method | Mutates _isMinimized |
Fires OnMinimized |
Spacer stays correct? |
|---|---|---|---|---|
| Manual minimize/restore button | ToggleMinimized() (line 179) |
yes | yes (line 182) | yes |
| Track selected → auto-expand | Expand() (line 118) via PlayerService.OnTrackSelected |
yes (true→false) |
no | no |
| Close (X) button | Close() (line 186) |
yes (false→true) |
no | no |
Resulting defects:
- Select a track: bar expands, spacer stays
minimized→ content overlaps the now-visible dock. - Close the player: bar collapses to the FAB, spacer stays
expanded→ a dead gap is left below content.
The ResizeObserver hook in OnAfterRenderAsync (line 83) already reacts to
_isMinimized correctly on every path — it toggles observe/unobserve purely off
!_isMinimized && !Fixed and guards re-entry with _spacerObserved. So the CSS-var
height publishing is not part of this bug; only the spacer's class (which reserves
the box at all) drifts. Any fix must preserve the observer hook's correctness.
Secondary observation (matters for Option B)
Close() is not a pure "minimize" — it also calls PlayerService.Unload() when a track
is loaded (line 188-190). The close path therefore couples two concerns: tearing down
playback and collapsing the chrome. A fix should keep "minimized state" as the thing the
spacer tracks, independent of the unload side effect.
2. Design options
Option A — Fix at the call sites
Fire the existing callback from the two paths that currently skip it: add
OnMinimized.InvokeAsync(false) inside Expand() and OnMinimized.InvokeAsync(true)
inside Close(), guarded the same way ToggleMinimized already guards
(if (OnMinimized.HasDelegate)), and only when the state actually flips.
Change: Two added invocations inside AudioPlayerBar.razor.cs. No new types, no DI,
no signature changes. MainLayout and the OnMinimized contract are untouched.
Scope: Smallest possible. One file, ~4 lines.
Edge cases / risks:
Expand()andClose()areasyncbut currently neverawait— addingawait OnMinimized.InvokeAsync(...)makes them genuinely async; confirm callers (PlayerService.OnTrackSelected = new EventCallback(this, Expand)at line 75, and the@onclickon the X button) await/route the returned task.EventCallbackhandles this correctly, so this is low risk but worth a glance.- Must fire only on an actual transition (mirror the existing
if (_isMinimized)/if (!_isMinimized)guards) to avoid redundantStateHasChangedchurn inMainLayout. - Keeps the bug class alive structurally: any future fourth path that sets
_isMinimizedwill reintroduce the same drift. This is a "patch each leak" fix, not a "remove the leak class" fix. - The contract stays push-based and one-directional (child → layout), which matches the
current mental model and the existing
ToggleMinimizedprecedent.
Option B — Lift minimize state into a shared service / cascade
Make _isMinimized a derived view of authoritative state held outside the bar — e.g. a
small PlayerChromeState service (scoped, cascaded by AudioPlayerProvider alongside the
player service) exposing IsMinimized + a StateChanged event. AudioPlayerBar reads
and writes through it; MainLayout subscribes (or cascades it) and derives
_audioPlayerClass reactively. The OnMinimized parameter is retired.
Change: New service type + DI registration; AudioPlayerBar rewires its three
mutation points to the service; MainLayout subscribes instead of taking a callback.
Touches 3-4 files plus Startup.ConfigureDomainServices.
Scope: Medium. Structural — introduces a new piece of shared state.
Edge cases / risks:
- Eliminates the bug class: any path that flips minimize state flows through one setter, so the spacer can never drift regardless of how many triggers exist later.
- Aligns with the project's stated direction — one source, multiple views: chrome state becomes a single authority that both the bar and the layout render from, rather than the bar owning private state and pushing notifications. (See memory: one-source-multiple-views.)
- The
ResizeObserverhook still keys off the bar's local read ofIsMinimized, so it stays correct — but the bar mustStateHasChanged(and thus re-runOnAfterRenderAsync) when the service fires, exactly as it already does forStateChangedfrom the player service (line 76-81). The wiring pattern already exists; this reuses it. - More moving parts for a defect that is, today, a missed callback. Risk of over-building if minimize state never grows beyond this one consumer.
- Must decide ownership lifetime: the cascade is
IsFixed(line: provider stores player withIsFixed="true"), so a cascaded chrome-state value would need the same subscribe-to-event escape hatch the bar already uses for the player service. Reuses a known pattern, but it is the fiddly part.
Option C — Single internal mutator (middle path) — recommended
Keep the OnMinimized callback contract and MainLayout exactly as they are, but funnel
all three trigger paths through one private method inside AudioPlayerBar that is the
only place _isMinimized is assigned:
SetMinimized(bool value):
if (_isMinimized == value) return; // transition guard
_isMinimized = value;
if (OnMinimized.HasDelegate) await OnMinimized.InvokeAsync(value);
StateHasChanged();
ToggleMinimized becomes SetMinimized(!_isMinimized). Expand() becomes
SetMinimized(false). Close() keeps its Unload() side effect, then calls
SetMinimized(true). OnParametersSet's Fixed branch (line 59-62) can also route
through it for consistency.
Change: One new private method; the three existing mutation points delegate to it.
Single file (AudioPlayerBar.razor.cs), no new types, no DI, no contract change.
Scope: As small as Option A, but structurally closes the bug class the way Option B does — by guaranteeing there is exactly one assignment site that always fires the bridge.
Why this is the sweet spot: It gets Option B's invariant ("every state change propagates") without Option B's new shared-state machinery. The drift is impossible because the callback firing is co-located with the only assignment, not duplicated across call sites where the next contributor can forget it.
3. Recommendation
Option C. It satisfies all three evaluation criteria with the smallest durable footprint:
- (a) Blast radius: one file, no new types, no DI, no contract or
MainLayoutchange — identical reach to Option A. - (b) Correctness across all three paths: by construction, every assignment of
_isMinimizedgoes through the single mutator that fires the bridge and guards the transition. Option A fixes today's two missed paths but leaves the pattern that produced them; Option C removes the pattern. - (c) ResizeObserver stays correct: the mutator calls
StateHasChanged(), which triggersOnAfterRenderAsync, which re-evaluatesshouldObserveoff the now-updated_isMinimizedexactly as today. No change to the observer logic.
Option B is the right move only if minimize/chrome state grows additional consumers
(e.g. a second surface that needs to read or drive it, or deep-link/restore-on-load
behaviour). It is over-scoped for the current single-consumer reality. Note the path
forward is clean: if that need arrives, Option C's single mutator is the natural seam to
later back with a service — SetMinimized becomes the one call site to redirect. Choosing
C now does not foreclose B later.
Trade-off being accepted: C keeps minimize state private to the bar and the sync push-based. If a future feature needs the layout (or anything else) to drive minimize state rather than just react to it, that is the trigger to revisit Option B.
4. Acceptance criteria
A correct fix satisfies all of the following observable conditions:
- Track-select expand: selecting a track while minimized expands the bar and the
spacer element's class changes from
minimizedtoexpanded(content reflows below the dock, no overlap). - Manual toggle sync: clicking minimize collapses the bar to the FAB and the spacer
class returns to
minimized; clicking restore expands the bar and the spacer class becomesexpanded. Bar and spacer never disagree. - Close sync: clicking the X minimizes the bar and the spacer class reverts to
minimized(no residual empty gap), and — unchanged — unloads the track when one is loaded. - No double-invocation: the manual toggle path fires
OnMinimizedexactly once per click (no regression from the existing single-fire behaviour). Re-triggering a path that does not change state (e.g.Expand()when already expanded) fires nothing. - ResizeObserver lifecycle: across all three paths, the spacer
ResizeObserverisobserved when (and only when) the bar is expanded and notFixed, andunobserved otherwise — i.e._spacerObservedends up consistent with!_isMinimized && !Fixedafter each transition. - No spacer height strand: after close/minimize, no stale
--player-heightreserves phantom space (the existingunobservepath already clears this; the fix must not bypass it).
5. Notes for the implementer (staff-engineer)
- The
Fixedembed path (line 59-62) sets_isMinimized = falsedirectly inOnParametersSetand intentionally renders noPlayerWindowControls(line 42-45) and no spacer observation (the!Fixedguard). Whatever mutator is introduced must not fireOnMinimizedfor theFixedembed in a way that makes a host page's layout reserve dock space it does not have. Simplest: theFixedbranch may bypass the callback, orMainLayoutis simply not present on embed surfaces. Confirm before routing theFixedbranch through the shared mutator. - This brief specifies behaviour and structure only. No implementation is included by design; dispatch staff-engineer to land it.