diff --git a/README.md b/README.md index 30be34b..4301d08 100644 --- a/README.md +++ b/README.md @@ -376,6 +376,9 @@ Then put it behind your existing reverse proxy like everything else it was bucketing by display name instead of Emby's own SortName, so an article-stripped title ("The ...") threw a whole run off — see docs/architecture.md +- [x] Fixed the music bar's queue popup — a missing `position` broke + the desktop layout when opened and left it unclosable on mobile + (its own close button was reachable now) — see docs/architecture.md - [ ] Try a full party end-to-end with two real accounts (still the main gap) - [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is confirmed here — no real device/session) diff --git a/client/src/musicBar/MusicBar.css b/client/src/musicBar/MusicBar.css index c802df8..ed6d833 100644 --- a/client/src/musicBar/MusicBar.css +++ b/client/src/musicBar/MusicBar.css @@ -13,6 +13,7 @@ sits flush above this bar's own actual rendered height (its content can wrap taller on a narrow screen), never a hardcoded guess at it. */ .musicBar { + position: relative; /* containing block for .musicBarQueue's own position: absolute - see its comment */ background: var(--bg-header); border-top: 1px solid rgba(255, 255, 255, .08); box-shadow: 0 -4px 16px rgba(0, 0, 0, .3); @@ -246,14 +247,26 @@ } /* Pops up ABOVE the bar (same pattern as Player.css's .settingsPanel above - its gear button) - collapsed by default, see MusicBar.ts's toggleQueueBtn - handler. Spans the full bar width (no max-width) rather than a fixed - popup box - a party's queue can run long, and a narrow column wastes - the width a desktop viewport already has going spare. - display:flex/flex-direction:column here (rather than on .pageHeader/ - #queueList individually) is what lets #queueList be the only child - that scrolls below - see #queueList's own comment. */ + its gear button - position: absolute against .musicBar's own + position: relative, bottom: 100% instead of that one's top: 100%/ + right: 0, since this floats up from a bottom bar rather than down from + a top-corner icon) - collapsed by default, see MusicBar.ts's + toggleQueueBtn handler. `position: absolute` (not left in normal flow) + is what stops opening this from silently growing .bottomStack's own + fixed-to-viewport height and shoving the actual controls upward + instead of floating over them - reported directly as the desktop bar + looking broken the moment the queue was opened. Spans the full bar + width (no max-width) rather than a fixed popup box - a party's queue + can run long, and a narrow column wastes the width a desktop viewport + already has going spare. display:flex/flex-direction:column here + (rather than on .pageHeader/#queueList individually) is what lets + #queueList be the only child that scrolls below - see #queueList's own + comment. */ .musicBarQueue { + position: absolute; + left: 0; + right: 0; + bottom: 100%; max-height: 50vh; margin: 0 16px; padding: 14px 14px 0; @@ -267,6 +280,23 @@ .musicBarQueue .pageHeader { flex-shrink: 0; } +/* Always reachable regardless of z-index/positioning below (see + MusicBar.ts's own click-outside handler for the other way to close + this on desktop) - reported directly that, in mobile's full-screen + layout, the *only* previous way to close this (tap the same + #toggleQueueBtn again) sat visually underneath the now-open queue + panel itself once it covered the screen, making it unreachable. */ +.queueCloseBtn { + background: none; + color: var(--muted); + font-size: 12px; + font-weight: 400; + padding: 2px; + flex-shrink: 0; +} + +.queueCloseBtn:hover { background: none; color: var(--text); } + /* The only scrolling region inside .musicBarQueue - "File d'attente" above it stays put instead of scrolling away with the tracks. */ #queueList { @@ -323,13 +353,27 @@ .musicBarPanelSide { margin-top: auto; justify-content: center; gap: 20px; } - /* Positioned + given a real z-index only here: a normal-flow sibling - otherwise loses to .musicBarPanel's fixed overlay above it once - that's open (its own z-index applies regardless of DOM order) - - opening the queue while the now-playing panel is expanded needs - to still show it on top, matching desktop's existing "queue pops - up above the bar" behavior. */ - .musicBarQueue { position: relative; z-index: 26; margin: 0; border-radius: 0; } + /* A full-screen overlay of its own here, not desktop's floating popup + (there's no "above the bar" to float over once .musicBarPanel is + itself a full-screen overlay) - z-index: 26 (above .musicBarPanel's + 25) is still what puts it on top, but `position: relative` used to + leave it in .musicBar's own normal flow, sized by `max-height: 50vh` + inherited from the base rule above and positioned whichever the + surrounding layout happened to allow - overlapping the very + #toggleQueueBtn used to open it, with no other way to close a + `position: relative` sibling that isn't just a small popup here. + `inset: 0` + `max-height: none` makes it genuinely full-screen like + .musicBarPanel itself, so its own #queueCloseBtn is always reachable + regardless of queue length. */ + .musicBarQueue { + position: fixed; + inset: 0; + z-index: 26; + max-height: none; + margin: 0; + border-radius: 0; + padding: 20px 20px calc(20px + env(safe-area-inset-bottom, 0px)); + } } /* One queue entry - a plain flex row (image, then a shrinking/truncating diff --git a/client/src/musicBar/MusicBar.html b/client/src/musicBar/MusicBar.html index 9dc8826..88a8e54 100644 --- a/client/src/musicBar/MusicBar.html +++ b/client/src/musicBar/MusicBar.html @@ -1,6 +1,11 @@
diff --git a/client/src/musicBar/MusicBar.ts b/client/src/musicBar/MusicBar.ts index e9a7165..f71a039 100644 --- a/client/src/musicBar/MusicBar.ts +++ b/client/src/musicBar/MusicBar.ts @@ -106,6 +106,7 @@ export function initMusicBar(root: HTMLElement): void { const voteSkipBtn = barEl.querySelector('#voteSkipBtn') as HTMLButtonElement; const toggleQueueBtn = barEl.querySelector('#toggleQueueBtn')!; const queuePanel = barEl.querySelector('#musicBarQueue')!; + const queueCloseBtn = barEl.querySelector('#queueCloseBtn')!; const leaveBtn = barEl.querySelector('#leaveBtn')!; const trackProgress = barEl.querySelector('#trackProgress') as HTMLElement; const mobileMiniBar = barEl.querySelector('#mobileMiniBar')!; @@ -120,6 +121,21 @@ export function initMusicBar(root: HTMLElement): void { miniPlayPauseBtn.addEventListener('click', () => conn?.setMusicPaused(!activeDriver!.paused)); voteSkipBtn.addEventListener('click', () => conn?.voteSkip()); toggleQueueBtn.addEventListener('click', () => queuePanel.classList.toggle('hide')); + queueCloseBtn.addEventListener('click', () => queuePanel.classList.add('hide')); + // Desktop-popover convention (same as views/player/Player.ts's own + // settingsPanel) - closes on any click that lands neither inside the + // panel nor on the button that opens it. Harmless on mobile's + // full-screen layout too (there's no "outside" to click there, but + // #queueCloseBtn above is the real fix for that case - see + // MusicBar.css's own comment on why the old position: relative left + // that button unreachable once the queue was open). + document.addEventListener('click', (e) => { + if (queuePanel.classList.contains('hide')) return; + const target = e.target as Node; + if (!queuePanel.contains(target) && !toggleQueueBtn.contains(target)) { + queuePanel.classList.add('hide'); + } + }); leaveBtn.addEventListener('click', () => leaveMusicParty()); // Mobile only (see MusicBar.css's 720px breakpoint - .musicBarPanel is diff --git a/docs/architecture.md b/docs/architecture.md index 616f5ef..63a6c96 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -4103,6 +4103,63 @@ it produced the same `A, B, T, B, L` heading sequence against the old bucket-by-`name` code, then confirmed the fix collapses it to the correct `A, B, L` against the exact same input. +## The music bar's queue popup: broken on desktop, unclosable on mobile + +Two reports that turned out to be the same missing `position` rule. +`.musicBarQueue`'s own comment already said it should "pop up ABOVE the +bar" (like `Player.css`'s `.settingsPanel` above its gear button), but +the CSS itself never actually gave it a `position` on desktop at all - +a plain normal-flow block, first child of `.musicBar`. Toggling it +visible didn't float anything; it pushed `#musicBarPanel` (the actual +transport controls) out of the way, growing `.bottomStack`'s own +fixed-to-viewport box from the bottom up - reported directly as the +player looking broken the moment the queue was opened. + +Mobile had a different-looking but related bug. `#musicBarPanel` +becomes a full-screen `position: fixed` overlay under 720px; a normal- +flow `.musicBarQueue` sibling would have rendered *behind* it, so it +had already been given `position: relative; z-index: 26` specifically +to still show on top once opened from inside that full-screen panel +(see the git history for that rule's own reasoning). That's a real fix +for visibility, but not for layout: `position: relative` leaves an +element sized and placed by normal flow, which put the queue somewhere +in the middle of the screen rather than covering it - directly +overlapping the very `#toggleQueueBtn` used to open it, the only way +that existed to close it again. Reported directly as "can't be closed." + +Fixed with a `position` on both sides, matching each context's actual +shape - `.musicBar` gets `position: relative` (a containing block), +desktop's `.musicBarQueue` becomes `position: absolute; left: 0; +right: 0; bottom: 100%` (floats directly above the bar, out of normal +flow, without touching `#musicBarPanel`'s own layout at all), and +mobile's becomes `position: fixed; inset: 0` (a proper full-screen +overlay of its own, `max-height: none` overriding the base rule's +`50vh` so it isn't clipped) - the same "size for what this context +actually needs" as `#musicBarPanel`'s own fixed/inset:0 treatment right +next to it. + +Also added, so closing it never again depends on precisely where the +toggle button happens to render underneath: a dedicated `#queueCloseBtn` +inside the queue's own header, plus a click-outside-closes listener on +`document` (`views/player/Player.ts`'s own `settingsPanel` pattern) for +the desktop popover case. + +Verified: full `typecheck`/`build` (client only). No Emby session here, +so checked directly - `initMusicBar` called against a synthetic +`.bottomStack`/`#musicBar` reproducing the real Shell markup, real +compiled CSS, a fake queue list: on the (real, 1280px) desktop viewport, +opening the queue landed its bottom edge exactly on the bar's own top +edge (zero gap, zero overlap) without moving the bar at all, and both +the close button and a click on unrelated page content behind it closed +it correctly. On a 375px-wide `