docs: move AudioPlayerBar unification from PLAN.md to COMPLETED.md
This commit is contained in:
@@ -8,6 +8,28 @@ Newest entries at the top. Group by phase/wave header (mirroring `PLAN.md` / `CM
|
||||
|
||||
## Phase 2 — Product surface: player and theming
|
||||
|
||||
**Status:** AudioPlayerBar responsive unification and SpectrumVisualizer fix landed on 2026-06-05.
|
||||
|
||||
### AudioPlayerBar Responsive Unification
|
||||
|
||||
**Landed 2026-06-05.**
|
||||
|
||||
Collapsed the two divergent Razor trees in `AudioPlayerBar.razor` (`@if (_isDesktop)` / `@else`) into a single markup tree where CSS — not a runtime breakpoint flag — drives the responsive layout. Removed `IBrowserViewportService`, the `_isDesktop` field, `OnAfterRenderAsync`, and the viewport subscription/unsubscription from the code-behind.
|
||||
|
||||
**Structural changes:**
|
||||
- Single `.player-layout` flex container (in `AudioPlayerBar.razor.css`) replaces the dual-branch conditional. Three children (`PlayerTransportZone`, `VolumeControls`, `PlayerSeekZone`) in source order; media query at 600px (`Sm` breakpoint) reorders via CSS `order` property and forces `SeekZone` to full-width below the transport/volume row on narrow viewports.
|
||||
- `PlayerTransportZone` flips its internal axis (vertical ↔ horizontal) via scoped CSS override of `MudStack` `flex-direction` at the 600px boundary — no parameter added to the component.
|
||||
- `::deep` prefix removed from `MudBlazor` component-class selectors in `PlayerTransportZone.razor.css` now that axis is purely CSS-driven and no runtime flag determines structure.
|
||||
- **SpectrumVisualizer bars now appear on first expand** — fixed by subscribing to the multicast `StateChanged` event (same pattern used by `AudioPlayerBar`), ensuring animation is initialized after mount.
|
||||
|
||||
**Scope:**
|
||||
- Unified responsive layout (desktop/mobile branches merged into single tree).
|
||||
- Both `AudioPlayerBar` and `SpectrumVisualizer` components affected.
|
||||
- Build clean: 0 errors, 0 new warnings.
|
||||
|
||||
**Notes for future work:**
|
||||
- First-render layout flash eliminated by construction (CSS media query evaluates at paint, not async subscription).
|
||||
|
||||
**Status:** Desktop AudioPlayerBar redesign landed on 2026-06-04.
|
||||
|
||||
### Desktop AudioPlayerBar — migrate to MudBlazor theme system
|
||||
|
||||
@@ -201,89 +201,3 @@ A small set of items that are real but don't fit a phase yet. Surface them when
|
||||
- **Mark genuinely uncertain items `[speculative]`** so future readers can tell what is direction vs. commitment.
|
||||
- **Open questions belong in the item that raises them**, not in a separate "questions" list — they expire when the item does.
|
||||
|
||||
---
|
||||
|
||||
## AudioPlayerBar Responsive Unification
|
||||
|
||||
**Goal:** Collapse the two divergent Razor trees in `AudioPlayerBar.razor` (`@if (_isDesktop)` / `@else`) into one markup tree where CSS — not a runtime breakpoint flag — does the responsive work. Removes the first-render layout flash caused by the async `BrowserViewportService` subscription, and deletes the inline duplication of the transport cluster that the mobile branch carries today.
|
||||
|
||||
This is presentation-layer cleanup, not a feature. It fits the `CONTEXT.md §6` "one source of truth, multiple views" principle applied at the markup level: one composition, two rendered shapes, divergence pushed entirely into rendering (CSS) rather than into a branch in the component.
|
||||
|
||||
### The core observation
|
||||
|
||||
The mobile branch's top row — `[PlayerControls + spinner] [Timestamp] [VolumeControls]` — is not a different set of controls from `PlayerTransportZone`. It *is* `PlayerTransportZone` (which already composes `PlayerControls` + spinner + `TimestampLabel`) laid out **horizontally instead of vertically**, with `VolumeControls` sitting beside it. The desktop branch already uses `PlayerTransportZone`; the mobile branch hand-rolls the same parts inline only because it needed a different axis and wanted volume adjacent.
|
||||
|
||||
So the entire desktop/mobile split reduces to two differences:
|
||||
|
||||
1. **`PlayerTransportZone` internal axis** — vertical (controls-over-timestamp) on wide, horizontal (controls-beside-timestamp) on narrow.
|
||||
2. **`VolumeControls` position** — far-right of the single row on wide; tucked beside the transport cluster on narrow (so the seek zone can take the full width below).
|
||||
|
||||
Both are achievable with one markup tree. Recommendation below; two alternatives considered after it.
|
||||
|
||||
### 1. Layout grid / arrangement (recommended)
|
||||
|
||||
One outer **flex container** that wraps, holding three children in source order: `PlayerTransportZone`, `VolumeControls`, `PlayerSeekZone`.
|
||||
|
||||
```
|
||||
┌─ .player-layout (flex, wrap) ──────────────────────────────────┐
|
||||
│ [PlayerTransportZone] [VolumeControls] [PlayerSeekZone ───] │
|
||||
└────────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
- **Wide (≥ breakpoint):** `flex-direction: row; flex-wrap: nowrap`. Transport at its natural `min-width`, VolumeControls at natural width, SeekZone `flex-grow: 1` eating the middle. To match today's desktop order (transport — seek — volume), SeekZone is given `order: 2` and VolumeControls `order: 3` at this breakpoint. Transport stays `order: 1`.
|
||||
- **Narrow (< breakpoint):** still `flex-wrap: wrap`, but SeekZone is forced to a new line via `flex-basis: 100%`. Transport (`order: 1`) and VolumeControls (`order: 2`) share the top line; SeekZone (`order: 3`, `flex-basis: 100%`) drops full-width below. This reproduces today's mobile shape exactly: `[transport] [volume]` over `[seek full-width]`.
|
||||
|
||||
The `order` swap is what lets a single source order serve both shapes. Source order is transport → volume → seek (the narrow reading order); `order` props rewrite it to transport → seek → volume on wide. Keeping source order = narrow order means the DOM reads naturally for the smaller, more constrained surface.
|
||||
|
||||
**Narrow-viewport answers to the brief's specific questions:**
|
||||
- *Where does VolumeControls go?* Beside the transport cluster, on the same top line — left of nothing, right of transport — exactly as today's mobile row. It does **not** drop below with the seek zone.
|
||||
- *Does PlayerTransportZone stay vertical or go horizontal?* It goes **horizontal** at narrow, so `[controls+spinner]` and `[timestamp]` sit side by side rather than stacked. This is the change that lets the transport cluster + volume fit on one line and matches the current mobile row.
|
||||
|
||||
### 2. PlayerTransportZone changes
|
||||
|
||||
`PlayerTransportZone` needs to flip its internal axis at the breakpoint. Two ways:
|
||||
|
||||
- **(Recommended) CSS-only, no new parameter.** The component's outer `MudStack Row="false"` renders a flex column. Scoped CSS in `PlayerTransportZone.razor.css` overrides `flex-direction` to `row` below the breakpoint via a media query, and adjusts `align-items` / gap to match. No C# parameter, no parent coordination — the component adapts to its own width context. This keeps the component self-contained and the parent ignorant of breakpoints.
|
||||
- Caveat: MudBlazor's `MudStack` emits inline-ish utility classes; overriding `flex-direction` on the rendered root needs a `::deep` rule with sufficient specificity (or a wrapper element the component controls). Confirm the generated class can be overridden cleanly; if not, fall back to wrapping the `MudStack` in a `div` the scoped CSS owns.
|
||||
|
||||
- **(Alternative) Add a `Row` (or `Vertical`) bool parameter.** Parent passes the axis explicitly. Rejected as the default because it reintroduces parent-side breakpoint awareness — the parent would need to know "narrow ⇒ Row=true," which is exactly the runtime-flag coupling we are removing. CSS-only keeps the responsive decision in CSS where the brief wants it. Keep this in pocket only if the `MudStack` override proves intractable.
|
||||
|
||||
Net: **prefer no parameter change to `PlayerTransportZone`.** CSS in its own scoped file handles the axis flip.
|
||||
|
||||
### 3. CSS strategy
|
||||
|
||||
- **`AudioPlayerBar.razor.css`** owns the *outer arrangement*: the `.player-layout` flex container, the `flex-wrap`, the `order` assignments, and the `flex-basis: 100%` line-break on the seek zone. This is where the three-zone composition lives, so its responsive rules belong here. Add a single media query block to the existing file (which already carries a `max-width: 768px` block for the dock/spacer — reuse the same threshold for consistency).
|
||||
- **`PlayerTransportZone.razor.css`** owns the *internal axis flip* of the transport cluster (column ⇄ row) and keeps its existing `min-width: 200px` on `.controls-left` (note: that `min-width` should be reviewed — at narrow widths a 200px floor on the transport cluster may crowd VolumeControls; consider relaxing it inside the narrow media query).
|
||||
- **Breakpoint threshold:** Use **`max-width: 599.98px`** to mirror MudBlazor's `Sm` boundary (600px), since the old `_isDesktop` flag used `Breakpoint.Sm` (`>= Sm` = desktop). This preserves the exact switch point users see today. Do **not** silently inherit the existing `768px` block's threshold for the *layout* switch — 768 is the dock-padding breakpoint and is a different concern; conflating them would move the layout switch point. (If Daniel prefers a single unified breakpoint for the whole component, that is a one-line decision — call it out, don't assume.)
|
||||
|
||||
Decision needed from Daniel: keep the layout switch at the historical 600px (`Sm`), or unify everything on 768px? Recommendation: keep 600px to preserve current behaviour; revisit only if the 200px transport floor forces an earlier wrap.
|
||||
|
||||
### 4. What `AudioPlayerBar.razor.cs` loses
|
||||
|
||||
All of the following are deleted — they exist only to drive the now-removed `@if (_isDesktop)` branch:
|
||||
|
||||
- The `private bool _isDesktop = true;` field.
|
||||
- The `private Guid _viewportSubscriptionId;` field.
|
||||
- The `[Inject] public required IBrowserViewportService BrowserViewportService { get; set; }` injection.
|
||||
- The entire `OnAfterRenderAsync(bool firstRender)` override (its only job is the breakpoint read + subscription).
|
||||
- The `BrowserViewportService.UnsubscribeAsync(_viewportSubscriptionId)` call inside `DisposeAsync` (the rest of `DisposeAsync` — the `StateChanged` detach — **stays**).
|
||||
- The `using MudBlazor.Services;` import, once `IBrowserViewportService` / `ResizeOptions` references are gone (verify nothing else in the file needs it — `using MudBlazor;` stays for `Color`, `Size`, etc.).
|
||||
|
||||
`IAsyncDisposable` stays on the class (still needed for the `StateChanged` unsubscribe); `DisposeAsync` remains but loses its viewport line. No other `.cs` logic changes — playback, seek, volume, minimize, and the `StateChanged` cascade plumbing are untouched.
|
||||
|
||||
### Razor consolidation (what the single tree looks like)
|
||||
|
||||
Replace the `@if (_isDesktop) { ... } else { ... }` block (lines 15–72) with one composition: the `.player-layout` flex container holding `PlayerTransportZone`, `VolumeControls`, and `PlayerSeekZone` in that source order. The mobile branch's hand-rolled `PlayerControls` + spinner + `TimestampLabel` + inline `VolumeControls` (lines 44–71) is deleted entirely — it is fully subsumed by `PlayerTransportZone` + the horizontal CSS flip. `PlayerWindowControls` (top-right minimize/close) and the error `MudAlert` are unchanged and sit outside `.player-layout` as they do today.
|
||||
|
||||
### Alternatives considered (for the outer layout)
|
||||
|
||||
- **(A) MudBlazor `MudGrid` / `MudItem` with `xs`/`sm` breakpoint props** instead of a hand-rolled flex container. MudBlazor's grid carries breakpoint props natively (`xs="12" sm="auto"`), which is idiomatic and avoids custom media queries for the *wrap*. Viable, and arguably the more "MudBlazor-native" move. Downside: the `order` swap (seek between transport and volume on wide, but below both on narrow) is awkward in a 12-column grid — you end up fighting column order with `order` utilities anyway, and the seek zone's full-width-on-narrow / grow-on-wide behaviour is cleaner as a flex `flex-grow` + `flex-basis` than as grid columns. Net: grid is tidy for the wrap but clumsy for the reorder. Flex wins on this specific layout.
|
||||
- **(B) CSS Grid with `grid-template-areas` swapped per breakpoint.** Most declarative — name the three areas and redraw the template in the media query. Cleanest conceptually, and the reorder is trivial (just rewrite the template). Downside: more CSS machinery than this three-element layout warrants, and `grid` interplay with MudBlazor component roots (which bring their own display) needs care. Reasonable if the flex `order` approach gets fiddly; otherwise heavier than needed.
|
||||
- **(Recommended) Flexbox with `order` + `flex-basis: 100%` line break.** Least machinery, maps directly onto the two differences identified, reuses the component's existing flex-based `MudStack` mental model. Recommended unless the `MudStack` override in §2 forces a rethink.
|
||||
|
||||
### Trade-offs / risks
|
||||
|
||||
- **MudStack override specificity.** The one real implementation risk is whether scoped `::deep` CSS can reliably override `MudStack`'s rendered `flex-direction`. If it can't, either wrap in a component-owned `div` (cheap) or fall back to the `Row` parameter (§2 alternative). Worth a 10-minute spike before committing the CSS-only path.
|
||||
- **Breakpoint divergence.** Two breakpoints now live in the component's CSS (600px for layout, 768px for dock padding). That's pre-existing for the padding; the layout one is new. If this bothers anyone, unify on one value — but that's a behaviour change, so it's Daniel's call, flagged above.
|
||||
- **No flash, by construction.** Because CSS evaluates at first paint (no async round-trip), the first-render flash is gone — this is the whole point and the primary win.
|
||||
- **`min-width: 200px` on `.controls-left`** may need relaxing at narrow widths once the transport cluster goes horizontal beside volume; check during implementation.
|
||||
|
||||
Reference in New Issue
Block a user