diff --git a/PLAN.md b/PLAN.md index 81dd5cb..b304d59 100644 --- a/PLAN.md +++ b/PLAN.md @@ -200,3 +200,90 @@ A small set of items that are real but don't fit a phase yet. Surface them when - **When something lands, move it to `COMPLETED.md`** rather than deleting it. Keep the original "What / Why / Shape" body intact so the history reads as a record of the decision, not just the outcome. - **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.