layout and remote fix
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 4m31s
Traceability Validation / Check Requirement Traces (push) Successful in 20s
Build & Release / Run Tests (push) Successful in 5m24s
🏗️ Build and Test JellyTau / Android Compile Check (push) Successful in 4m29s
Build & Release / Build Linux (push) Successful in 17m27s
Build & Release / Build Android (push) Successful in 22m14s
Build & Release / Create Release (push) Successful in 12s
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 4m31s
Traceability Validation / Check Requirement Traces (push) Successful in 20s
Build & Release / Run Tests (push) Successful in 5m24s
🏗️ Build and Test JellyTau / Android Compile Check (push) Successful in 4m29s
Build & Release / Build Linux (push) Successful in 17m27s
Build & Release / Build Android (push) Successful in 22m14s
Build & Release / Create Release (push) Successful in 12s
This commit is contained in:
@@ -0,0 +1,64 @@
|
||||
<!--
|
||||
BottomUi — the app's bottom UI (mini player stacked over the bottom nav).
|
||||
|
||||
Rendered as an IN-FLOW flex child at the bottom of a full-height flex column,
|
||||
NOT a fixed overlay. This is the whole point: because it is a normal flex
|
||||
sibling below the scroll container (which is `flex-1 min-h-0 overflow-y-auto`),
|
||||
the scroller is physically bounded above it and can never render behind it.
|
||||
|
||||
This replaces the old ResizeObserver + `bottomUiHeight` + padding-reservation
|
||||
scheme, which started at 0, updated async, and repeatedly regressed into the
|
||||
"last row hidden behind the nav" bug. There is nothing to measure or reserve:
|
||||
the browser's flex layout does it exactly, every frame.
|
||||
|
||||
The Android system gesture bar is cleared via `env(safe-area-inset-bottom)`.
|
||||
|
||||
TRACES: UR-005 | DR-009
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { goto } from "$app/navigation";
|
||||
import { currentMedia, isPlaying, playbackPosition, playbackDuration } from "$lib/stores/player";
|
||||
import { isShuffle, repeatMode, hasNext, hasPrevious } from "$lib/stores/queue";
|
||||
import { showSleepTimerModal } from "$lib/stores/appState";
|
||||
import MiniPlayer from "$lib/components/player/MiniPlayer.svelte";
|
||||
import BottomNav from "$lib/components/BottomNav.svelte";
|
||||
|
||||
let {
|
||||
showMiniPlayer = true,
|
||||
showNav = true,
|
||||
onExpand,
|
||||
}: {
|
||||
showMiniPlayer?: boolean;
|
||||
showNav?: boolean;
|
||||
// Where "expand mini player" goes. Defaults to the full player route.
|
||||
onExpand?: () => void;
|
||||
} = $props();
|
||||
|
||||
function expand() {
|
||||
if (onExpand) return onExpand();
|
||||
if ($currentMedia) goto(`/player/${$currentMedia.id}`);
|
||||
}
|
||||
</script>
|
||||
|
||||
<!-- flex-shrink-0 so it keeps its natural height; the scroller sibling flexes. -->
|
||||
<div class="flex-shrink-0 pb-[env(safe-area-inset-bottom)] bg-[var(--color-surface)]">
|
||||
{#if showMiniPlayer}
|
||||
<MiniPlayer
|
||||
media={$currentMedia}
|
||||
isPlaying={$isPlaying}
|
||||
position={$playbackPosition}
|
||||
duration={$playbackDuration}
|
||||
shuffle={$isShuffle}
|
||||
repeat={$repeatMode}
|
||||
hasNext={$hasNext}
|
||||
hasPrevious={$hasPrevious}
|
||||
className="flex-shrink-0"
|
||||
onExpand={expand}
|
||||
onSleepTimerClick={() => showSleepTimerModal.set(true)}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
{#if showNav}
|
||||
<BottomNav className="flex-shrink-0" />
|
||||
{/if}
|
||||
</div>
|
||||
@@ -9,10 +9,6 @@ export const isAndroid = writable(false);
|
||||
// Shuffle/repeat/next/previous state now lives in the event-driven queue store
|
||||
// ($lib/stores/queue), the single source of truth.
|
||||
export const showSleepTimerModal = writable(false);
|
||||
// Measured height (px) of the fixed bottom UI on Android: BottomNav stacked with
|
||||
// the global mini player. Published by the root layout via ResizeObserver so the
|
||||
// library list can reserve exactly that much bottom padding (no magic rem guesses).
|
||||
export const bottomUiHeight = writable(0);
|
||||
|
||||
// Library-specific state
|
||||
export const librarySearchQuery = writable("");
|
||||
|
||||
@@ -26,6 +26,18 @@ vi.mock("./sessions", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
// Capture the playerStatusEvent listener so tests can drive backend
|
||||
// `playback_mode_changed` events through the reconciler. The commands still flow
|
||||
// to the real bindings (which call the mocked `invoke`), so the existing
|
||||
// refresh/transfer tests keep exercising the true command path.
|
||||
let capturedStatusListener: ((event: { payload: any }) => void) | null = null;
|
||||
vi.mock("@tauri-apps/api/event", () => ({
|
||||
listen: vi.fn((_name: string, cb: (event: { payload: any }) => void) => {
|
||||
capturedStatusListener = cb;
|
||||
return Promise.resolve(() => {});
|
||||
}),
|
||||
}));
|
||||
|
||||
// Mock auth store
|
||||
const mockGetHandle = vi.fn(() => "repo-handle-1");
|
||||
vi.mock("./auth", () => ({
|
||||
@@ -42,6 +54,7 @@ describe("playbackMode store", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
currentSelectedSession = null;
|
||||
capturedStatusListener = null;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -327,6 +340,56 @@ describe("playbackMode store", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("backend playback_mode_changed reconciler", () => {
|
||||
// Regression: commit 2a1f168 made Rust re-broadcast PlaybackModeChanged on
|
||||
// every set_mode. Local playback drives set_mode("local") from both the
|
||||
// frontend and Rust, so the same mode arrives repeatedly. The reconciler
|
||||
// used to run selectSession(null) on each one, deselecting the remote
|
||||
// session mid-cast and tripping the disconnect watchdog — which broke the
|
||||
// lockscreen card, remote volume, and (via the mode flap) local audio.
|
||||
async function initListener() {
|
||||
const { playbackMode } = await import("./playbackMode");
|
||||
playbackMode.initializeSessionMonitoring();
|
||||
expect(capturedStatusListener).not.toBeNull();
|
||||
return playbackMode;
|
||||
}
|
||||
|
||||
it("ignores a no-op remote re-broadcast (no session churn)", async () => {
|
||||
currentSelectedSession = { id: "sess-1" };
|
||||
const playbackMode = await initListener();
|
||||
playbackMode.setMode("remote", "sess-1");
|
||||
mockSelectSession.mockClear();
|
||||
|
||||
// Rust re-broadcasts the SAME remote mode (e.g. a position tick path).
|
||||
capturedStatusListener!({
|
||||
payload: { type: "playback_mode_changed", mode: "remote", session_id: "sess-1" },
|
||||
});
|
||||
|
||||
const state = get(playbackMode);
|
||||
expect(state.mode).toBe("remote");
|
||||
expect(state.remoteSessionId).toBe("sess-1");
|
||||
// Must NOT re-select (which would churn the watchdog) on a no-op.
|
||||
expect(mockSelectSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("adopts a genuine remote→local change and clears the session", async () => {
|
||||
currentSelectedSession = { id: "sess-1" };
|
||||
const playbackMode = await initListener();
|
||||
playbackMode.setMode("remote", "sess-1");
|
||||
mockSelectSession.mockClear();
|
||||
|
||||
capturedStatusListener!({
|
||||
payload: { type: "playback_mode_changed", mode: "local", session_id: null },
|
||||
});
|
||||
|
||||
const state = get(playbackMode);
|
||||
expect(state.mode).toBe("local");
|
||||
expect(state.remoteSessionId).toBeNull();
|
||||
expect(mockSelectSession).toHaveBeenCalledWith(null);
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe("transfer reconciles to Rust on completion", () => {
|
||||
it("refreshes from Rust after a successful transferToRemote", async () => {
|
||||
const { playbackMode } = await import("./playbackMode");
|
||||
|
||||
@@ -316,10 +316,31 @@ function createPlaybackModeStore() {
|
||||
const mode = event.payload.mode as PlaybackMode;
|
||||
const remoteSessionId =
|
||||
mode === "remote" ? event.payload.session_id ?? null : null;
|
||||
|
||||
// Ignore no-op re-broadcasts. The backend re-emits on every set_mode, and
|
||||
// local playback drives set_mode("local") from BOTH the frontend
|
||||
// (handleStateChanged) and Rust, so the same mode arrives repeatedly. If
|
||||
// we reconciled unconditionally we'd re-run selectSession(null) on each
|
||||
// one, deselecting the remote session mid-cast and tripping the
|
||||
// disconnect-to-idle watchdog (breaking the lockscreen card, remote
|
||||
// volume, and — via the resulting mode flap — local audio).
|
||||
if (
|
||||
currentState.mode === mode &&
|
||||
currentState.remoteSessionId === remoteSessionId
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
console.log("[PlaybackMode] Backend mode changed →", mode, remoteSessionId);
|
||||
update((s) => ({ ...s, mode, remoteSessionId }));
|
||||
// Keep the selected session in step so the merged UI stores follow.
|
||||
sessions.selectSession(remoteSessionId);
|
||||
// Keep the selected session in step so the merged UI stores follow, but
|
||||
// only touch the selection when it actually differs — re-selecting the
|
||||
// same id (or clearing on a non-remote emit that isn't a real change)
|
||||
// would needlessly churn the session watchdog.
|
||||
const selected = get(selectedSession);
|
||||
if ((selected?.id ?? null) !== remoteSessionId) {
|
||||
sessions.selectSession(remoteSessionId);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -1,13 +1,15 @@
|
||||
/**
|
||||
* Regression tests for the app's fixed bottom-UI (mini player + bottom nav)
|
||||
* layout rules.
|
||||
* Tests for the app's bottom-UI (mini player + bottom nav) visibility rules.
|
||||
*
|
||||
* The bug these guard against: on the library route the layout used to render
|
||||
* its OWN in-flow mini player while the root ALSO painted a fixed bottom nav on
|
||||
* top of it, and the library scroller only reserved 1rem — so the last row hid
|
||||
* behind the nav. The fix unified everything onto the root: the root owns the
|
||||
* single fixed bottom UI on every route/platform, and every scroll container
|
||||
* reserves the measured `bottomUiHeight`.
|
||||
* The overlap bug these guard against: on the library page the last rows were
|
||||
* hidden behind the bottom nav. It was caused by rendering the bottom UI as a
|
||||
* FIXED overlay and trying to reserve its (async-measured, initially-0) height
|
||||
* as padding. The fix renders the bottom UI as an in-flow flex child below the
|
||||
* scroller, so overlap is structurally impossible — no measurement, no padding.
|
||||
*
|
||||
* These pure functions only decide *whether* each piece shows on a route. The
|
||||
* structural guarantee (flex sibling below the scroller) is exercised by
|
||||
* running the app, not by jsdom (which has no layout engine).
|
||||
*
|
||||
* TRACES: UR-005 | DR-009
|
||||
*/
|
||||
@@ -18,7 +20,6 @@ import {
|
||||
showGlobalMiniPlayer,
|
||||
routeOwnsLayout,
|
||||
showBottomUi,
|
||||
reservedBottomPadding,
|
||||
} from "./layoutShell";
|
||||
|
||||
const authed = (pathname: string) => ({ pathname, isAuthenticated: true });
|
||||
@@ -44,15 +45,14 @@ describe("showGlobalMiniPlayer", () => {
|
||||
expect(showGlobalMiniPlayer({ pathname: "/settings" })).toBe(false);
|
||||
});
|
||||
|
||||
it("does NOT depend on platform or on /library — the root owns it everywhere", () => {
|
||||
// The signature intentionally has no `isAndroid` input: the old bug was a
|
||||
// platform/route split that let a second in-flow mini player exist.
|
||||
it("does NOT depend on platform or on /library — one code path everywhere", () => {
|
||||
// The old bug was a platform/route split that let a second mini player exist.
|
||||
expect(showGlobalMiniPlayer({ pathname: "/library" })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("showBottomNav", () => {
|
||||
it("shows on authenticated content routes including library", () => {
|
||||
it("shows on authenticated content routes including library and settings", () => {
|
||||
expect(showBottomNav(authed("/library"))).toBe(true);
|
||||
expect(showBottomNav(authed("/"))).toBe(true);
|
||||
expect(showBottomNav(authed("/settings"))).toBe(true);
|
||||
@@ -69,64 +69,55 @@ describe("showBottomNav", () => {
|
||||
});
|
||||
|
||||
describe("routeOwnsLayout", () => {
|
||||
it("is true for library/settings/player/login (they manage their own scroll)", () => {
|
||||
it("is true for library/player/login (they render their own flex column + BottomUi)", () => {
|
||||
expect(routeOwnsLayout({ pathname: "/library" })).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/library/abc" })).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/settings" })).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/player/x" })).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/login" })).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for routes that render into the root scroller", () => {
|
||||
it("is false for routes that render into the root scroller (incl. settings)", () => {
|
||||
// Settings has no +layout of its own; it must flow through the root scroller
|
||||
// so the root's in-flow BottomUi renders below it (otherwise settings loses
|
||||
// its nav, since the fixed-overlay nav no longer exists).
|
||||
expect(routeOwnsLayout({ pathname: "/" })).toBe(false);
|
||||
expect(routeOwnsLayout({ pathname: "/search" })).toBe(false);
|
||||
expect(routeOwnsLayout({ pathname: "/downloads" })).toBe(false);
|
||||
expect(routeOwnsLayout({ pathname: "/settings" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("layout invariant: a reservation owner exists wherever bottom UI shows", () => {
|
||||
// The core anti-regression check. Every route falls into exactly one of two
|
||||
// reservation regimes:
|
||||
// - route owns its layout -> the route's own scroller reserves bottomUiHeight
|
||||
// - route does NOT own it -> the root scroller reserves bottomUiHeight
|
||||
// The bug was that the library route was implicitly a THIRD regime: it owned
|
||||
// its layout, showed a fixed nav from the root, but reserved only 1rem. That
|
||||
// can't recur now because library both owns its layout (so it reserves
|
||||
// internally) and the mini player is root-owned (no second in-flow bar).
|
||||
describe("structural invariant: every route that shows bottom UI has a scroller above it", () => {
|
||||
// With the in-flow model, "the bottom UI is a flex sibling below a scroller"
|
||||
// must hold on every route where it shows. That scroller is provided by
|
||||
// exactly one owner:
|
||||
// - routeOwnsLayout === true -> the route's own column (header + main + BottomUi)
|
||||
// - routeOwnsLayout === false -> the root column (scroller + BottomUi)
|
||||
// The forbidden state — bottom UI shows but no owning column renders a
|
||||
// scroller + BottomUi pair — cannot occur because the two branches are total.
|
||||
const routes = ["/", "/search", "/downloads", "/library", "/library/abc", "/settings"];
|
||||
|
||||
for (const pathname of routes) {
|
||||
it(`${pathname}: exactly one reservation owner`, () => {
|
||||
if (!showBottomUi(authed(pathname))) return; // no bottom UI -> nothing to reserve
|
||||
// Ownership is a total boolean, so exactly one regime always applies —
|
||||
// there is no route that shows bottom UI with no reservation owner.
|
||||
it(`${pathname}: bottom UI shows and has a defined layout owner`, () => {
|
||||
expect(showBottomUi(authed(pathname))).toBe(true);
|
||||
expect(typeof routeOwnsLayout({ pathname })).toBe("boolean");
|
||||
});
|
||||
}
|
||||
|
||||
it("library shows bottom UI AND owns its layout, so it reserves internally", () => {
|
||||
// Directly pins the regression: library must NOT rely on the root scroller
|
||||
// (it has none — the root gives owning routes a clipped, non-scrolling box).
|
||||
it("library owns its layout, so it renders its own in-flow BottomUi", () => {
|
||||
// Directly pins the original regression: library must render BottomUi inside
|
||||
// its own column (the root gives owning routes a clipped, non-scrolling box).
|
||||
expect(showBottomUi(authed("/library"))).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/library" })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("reservedBottomPadding", () => {
|
||||
it("returns an exact px fit when no extra room requested", () => {
|
||||
expect(reservedBottomPadding(120)).toBe("120px");
|
||||
it("settings does NOT own its layout, so the root scroller + BottomUi cover it", () => {
|
||||
expect(showBottomUi(authed("/settings"))).toBe(true);
|
||||
expect(routeOwnsLayout({ pathname: "/settings" })).toBe(false);
|
||||
});
|
||||
|
||||
it("adds breathing room via calc for layout-owning routes", () => {
|
||||
expect(reservedBottomPadding(120, 1)).toBe("calc(120px + 1rem)");
|
||||
});
|
||||
|
||||
it("never returns negative padding", () => {
|
||||
expect(reservedBottomPadding(-50)).toBe("0px");
|
||||
expect(reservedBottomPadding(-50, 1)).toBe("calc(0px + 1rem)");
|
||||
});
|
||||
|
||||
it("reserves 1rem-only when the bottom UI is collapsed to 0 (nothing playing, nav-only measured elsewhere)", () => {
|
||||
expect(reservedBottomPadding(0, 1)).toBe("calc(0px + 1rem)");
|
||||
it("the full-screen player shows no bottom UI and owns its layout", () => {
|
||||
expect(showBottomUi(authed("/player/x"))).toBe(false);
|
||||
expect(routeOwnsLayout({ pathname: "/player/x" })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,20 +1,18 @@
|
||||
/**
|
||||
* Pure layout-shell logic for the app's fixed bottom UI (mini player stacked
|
||||
* over the bottom nav).
|
||||
* Pure layout-shell visibility rules for the app's bottom UI (mini player
|
||||
* stacked over the bottom nav).
|
||||
*
|
||||
* These rules used to live as inline `$derived` booleans scattered across the
|
||||
* root and library `+layout.svelte` files, and diverged per platform/route —
|
||||
* which is exactly how the "last row hidden behind the nav" bug kept coming
|
||||
* back. The invariant is now a single source of truth:
|
||||
* root and library `+layout.svelte` files and diverged per platform/route.
|
||||
*
|
||||
* - The ROOT layout owns the single fixed bottom UI on every route/platform.
|
||||
* There is no per-route/per-platform second mini player.
|
||||
* - Whatever fixed bottom UI is showing has a live-measured height
|
||||
* (`bottomUiHeight`), and every scroll container reserves exactly that much
|
||||
* bottom space so the last row can never render behind the nav.
|
||||
* The overlap bug ("last row hidden behind the nav") is now solved
|
||||
* STRUCTURALLY, not by these rules: the bottom UI is rendered as an in-flow
|
||||
* flex child below the scroller (see BottomUi.svelte), so the scroller is
|
||||
* physically bounded above it and can never render behind it. There is no
|
||||
* measurement and no reserved padding. These functions only decide *whether*
|
||||
* each piece is visible on a given route.
|
||||
*
|
||||
* Keeping this pure makes the invariant unit-testable (jsdom has no layout
|
||||
* engine, so the geometry itself can't be tested — but the decision logic can).
|
||||
* Keeping them pure makes the visibility contract unit-testable.
|
||||
*
|
||||
* TRACES: UR-005 | DR-009
|
||||
*/
|
||||
@@ -56,41 +54,24 @@ export function showGlobalMiniPlayer({ pathname }: { pathname: string }): boolea
|
||||
}
|
||||
|
||||
/**
|
||||
* Routes that own their own full-height layout (their own scroll container +
|
||||
* bottom-space reservation). The root leaves these as a plain non-scrolling box
|
||||
* and does NOT add bottom padding — the route reserves `bottomUiHeight` itself.
|
||||
* Every other route scrolls in the root wrapper, which reserves the space.
|
||||
* Routes that render their own full-height flex column (header + scroller +
|
||||
* their own in-flow BottomUi). The root leaves these as a plain clipped box and
|
||||
* does not render its own BottomUi. Every other route renders into the root's
|
||||
* scroller, with the root's in-flow BottomUi as a flex sibling below it.
|
||||
*/
|
||||
export function routeOwnsLayout({ pathname }: { pathname: string }): boolean {
|
||||
return (
|
||||
pathname.startsWith("/library") ||
|
||||
pathname.startsWith("/settings") ||
|
||||
pathname.startsWith("/player/") ||
|
||||
pathname.startsWith("/login")
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether any fixed bottom UI is showing for this route (mini player, nav, or
|
||||
* both). When true, the active scroll container must reserve `bottomUiHeight`.
|
||||
* Whether any bottom UI is showing for this route (mini player, nav, or both).
|
||||
* The bottom UI is rendered in flex flow below the scroller (see BottomUi.svelte),
|
||||
* so this is purely a visibility question — there is no padding to reserve.
|
||||
*/
|
||||
export function showBottomUi(input: BottomUiVisibilityInput): boolean {
|
||||
return showBottomNav(input) || showGlobalMiniPlayer({ pathname: input.pathname });
|
||||
}
|
||||
|
||||
/**
|
||||
* The bottom padding (in CSS) a scroll container must reserve so its last row
|
||||
* clears the fixed bottom UI. `bottomUiHeight` is the live-measured height of
|
||||
* the root's fixed bottom UI wrapper.
|
||||
*
|
||||
* @param bottomUiHeight measured height in px of the fixed bottom UI (0 if none)
|
||||
* @param extraRem breathing room added on top (routes that own their
|
||||
* layout add 1rem; the root wrapper reserves an exact fit)
|
||||
*/
|
||||
export function reservedBottomPadding(
|
||||
bottomUiHeight: number,
|
||||
extraRem = 0,
|
||||
): string {
|
||||
const px = Math.max(0, bottomUiHeight);
|
||||
return extraRem > 0 ? `calc(${px}px + ${extraRem}rem)` : `${px}px`;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user