Merge pull request 'Fix the music bar queue popup: broken layout on desktop, unclosable on mobile' (#17) from fix/music-bar-queue-popup into master
This commit is contained in:
commit
658ad1d01d
5 changed files with 139 additions and 14 deletions
|
|
@ -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,
|
it was bucketing by display name instead of Emby's own SortName,
|
||||||
so an article-stripped title ("The ...") threw a whole run off —
|
so an article-stripped title ("The ...") threw a whole run off —
|
||||||
see docs/architecture.md
|
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 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
|
- [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is
|
||||||
confirmed here — no real device/session)
|
confirmed here — no real device/session)
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@
|
||||||
sits flush above this bar's own actual rendered height (its content
|
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. */
|
can wrap taller on a narrow screen), never a hardcoded guess at it. */
|
||||||
.musicBar {
|
.musicBar {
|
||||||
|
position: relative; /* containing block for .musicBarQueue's own position: absolute - see its comment */
|
||||||
background: var(--bg-header);
|
background: var(--bg-header);
|
||||||
border-top: 1px solid rgba(255, 255, 255, .08);
|
border-top: 1px solid rgba(255, 255, 255, .08);
|
||||||
box-shadow: 0 -4px 16px rgba(0, 0, 0, .3);
|
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
|
/* Pops up ABOVE the bar (same pattern as Player.css's .settingsPanel above
|
||||||
its gear button) - collapsed by default, see MusicBar.ts's toggleQueueBtn
|
its gear button - position: absolute against .musicBar's own
|
||||||
handler. Spans the full bar width (no max-width) rather than a fixed
|
position: relative, bottom: 100% instead of that one's top: 100%/
|
||||||
popup box - a party's queue can run long, and a narrow column wastes
|
right: 0, since this floats up from a bottom bar rather than down from
|
||||||
the width a desktop viewport already has going spare.
|
a top-corner icon) - collapsed by default, see MusicBar.ts's
|
||||||
display:flex/flex-direction:column here (rather than on .pageHeader/
|
toggleQueueBtn handler. `position: absolute` (not left in normal flow)
|
||||||
#queueList individually) is what lets #queueList be the only child
|
is what stops opening this from silently growing .bottomStack's own
|
||||||
that scrolls below - see #queueList's own comment. */
|
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 {
|
.musicBarQueue {
|
||||||
|
position: absolute;
|
||||||
|
left: 0;
|
||||||
|
right: 0;
|
||||||
|
bottom: 100%;
|
||||||
max-height: 50vh;
|
max-height: 50vh;
|
||||||
margin: 0 16px;
|
margin: 0 16px;
|
||||||
padding: 14px 14px 0;
|
padding: 14px 14px 0;
|
||||||
|
|
@ -267,6 +280,23 @@
|
||||||
|
|
||||||
.musicBarQueue .pageHeader { flex-shrink: 0; }
|
.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"
|
/* The only scrolling region inside .musicBarQueue - "File d'attente"
|
||||||
above it stays put instead of scrolling away with the tracks. */
|
above it stays put instead of scrolling away with the tracks. */
|
||||||
#queueList {
|
#queueList {
|
||||||
|
|
@ -323,13 +353,27 @@
|
||||||
|
|
||||||
.musicBarPanelSide { margin-top: auto; justify-content: center; gap: 20px; }
|
.musicBarPanelSide { margin-top: auto; justify-content: center; gap: 20px; }
|
||||||
|
|
||||||
/* Positioned + given a real z-index only here: a normal-flow sibling
|
/* A full-screen overlay of its own here, not desktop's floating popup
|
||||||
otherwise loses to .musicBarPanel's fixed overlay above it once
|
(there's no "above the bar" to float over once .musicBarPanel is
|
||||||
that's open (its own z-index applies regardless of DOM order) -
|
itself a full-screen overlay) - z-index: 26 (above .musicBarPanel's
|
||||||
opening the queue while the now-playing panel is expanded needs
|
25) is still what puts it on top, but `position: relative` used to
|
||||||
to still show it on top, matching desktop's existing "queue pops
|
leave it in .musicBar's own normal flow, sized by `max-height: 50vh`
|
||||||
up above the bar" behavior. */
|
inherited from the base rule above and positioned whichever the
|
||||||
.musicBarQueue { position: relative; z-index: 26; margin: 0; border-radius: 0; }
|
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
|
/* One queue entry - a plain flex row (image, then a shrinking/truncating
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,11 @@
|
||||||
<div id="musicBarQueue" class="musicBarQueue hide">
|
<div id="musicBarQueue" class="musicBarQueue hide">
|
||||||
<div class="pageHeader">
|
<div class="pageHeader">
|
||||||
<h2>File d'attente</h2>
|
<h2>File d'attente</h2>
|
||||||
|
<button class="queueCloseBtn" id="queueCloseBtn" aria-label="Fermer la file d'attente">
|
||||||
|
<svg viewBox="0 0 24 24" width="20" height="20" aria-hidden="true">
|
||||||
|
<path d="M6 9l6 6 6-6" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"></path>
|
||||||
|
</svg>
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
<div id="queueList" class="trackList"><p class="hint">Rien en attente.</p></div>
|
<div id="queueList" class="trackList"><p class="hint">Rien en attente.</p></div>
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -106,6 +106,7 @@ export function initMusicBar(root: HTMLElement): void {
|
||||||
const voteSkipBtn = barEl.querySelector('#voteSkipBtn') as HTMLButtonElement;
|
const voteSkipBtn = barEl.querySelector('#voteSkipBtn') as HTMLButtonElement;
|
||||||
const toggleQueueBtn = barEl.querySelector('#toggleQueueBtn')!;
|
const toggleQueueBtn = barEl.querySelector('#toggleQueueBtn')!;
|
||||||
const queuePanel = barEl.querySelector('#musicBarQueue')!;
|
const queuePanel = barEl.querySelector('#musicBarQueue')!;
|
||||||
|
const queueCloseBtn = barEl.querySelector('#queueCloseBtn')!;
|
||||||
const leaveBtn = barEl.querySelector('#leaveBtn')!;
|
const leaveBtn = barEl.querySelector('#leaveBtn')!;
|
||||||
const trackProgress = barEl.querySelector('#trackProgress') as HTMLElement;
|
const trackProgress = barEl.querySelector('#trackProgress') as HTMLElement;
|
||||||
const mobileMiniBar = barEl.querySelector('#mobileMiniBar')!;
|
const mobileMiniBar = barEl.querySelector('#mobileMiniBar')!;
|
||||||
|
|
@ -120,6 +121,21 @@ export function initMusicBar(root: HTMLElement): void {
|
||||||
miniPlayPauseBtn.addEventListener('click', () => conn?.setMusicPaused(!activeDriver!.paused));
|
miniPlayPauseBtn.addEventListener('click', () => conn?.setMusicPaused(!activeDriver!.paused));
|
||||||
voteSkipBtn.addEventListener('click', () => conn?.voteSkip());
|
voteSkipBtn.addEventListener('click', () => conn?.voteSkip());
|
||||||
toggleQueueBtn.addEventListener('click', () => queuePanel.classList.toggle('hide'));
|
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());
|
leaveBtn.addEventListener('click', () => leaveMusicParty());
|
||||||
|
|
||||||
// Mobile only (see MusicBar.css's 720px breakpoint - .musicBarPanel is
|
// Mobile only (see MusicBar.css's 720px breakpoint - .musicBarPanel is
|
||||||
|
|
|
||||||
|
|
@ -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
|
bucket-by-`name` code, then confirmed the fix collapses it to the
|
||||||
correct `A, B, L` against the exact same input.
|
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 `<iframe>` (this sandbox's own
|
||||||
|
`resize_window` viewport emulation is unreliable - see the mobile-nav/
|
||||||
|
music-UI/library-grid sections above), with the now-playing panel
|
||||||
|
opened first: the queue rendered as a genuine full-viewport overlay,
|
||||||
|
`elementFromPoint` at the close button's own center hit the close button
|
||||||
|
itself (not something on top of it), and closing the queue correctly
|
||||||
|
returned to the still-open now-playing panel rather than closing
|
||||||
|
everything.
|
||||||
|
|
||||||
## Known limitations
|
## Known limitations
|
||||||
|
|
||||||
- Playback always goes through `hls.js` against whatever Emby's
|
- Playback always goes through `hls.js` against whatever Emby's
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue