fix(offline): keep the offline banner off the full-screen player

Every other shell rule in layoutShell.ts already treats /player/* as immersive; the amber "You're offline" strip was the one piece of chrome still rendered above it. On the native Android video path that is not cosmetic: VideoPlayer makes itself transparent so the ExoPlayer SurfaceView behind the WebView is visible (DR-185), so a shell child that still paints shows through the picture as a stripe across the top of the film. Offline is also precisely when a downloaded video plays, so the banner appeared when it was most in the way — and it offers the viewer nothing to act on, since local playback needs no server. The rule moves into the pure module as showOfflineBanner() rather than staying an inline {#if} in the shell, so the immersive-route contract is stated in one tested place. TRACES: UR-003, UR-043 | DR-291 | UT-255
2026-09-22 21:08:03 -04:00
parent 0f928798d7
commit b98cbfe28a
4 changed files with 70 additions and 4 deletions
@@ -492,6 +492,7 @@ Internal architecture, components, and application logic.
 | DR-288 | A type-filtered listing states `Recursive` explicitly. Jellyfin 12.0 defaults it to true when the parent is a library folder and `IncludeItemTypes` is set, where 10.11 returned immediate children — the identical request, a different result set, with nothing in the response to say which rule applied. Sending the value the client actually wants makes both generations agree, and the value sent is the one that shipped rather than the new server-side default, so this is a compatibility fix and not a silent behaviour change | Repository | UR-085 | Proposed |
 | DR-289 | The download worker's HTTP client carries a **read** timeout and a connect timeout, never a total request timeout. reqwest's `Client::timeout` is a deadline that runs until the body has finished, and it was set to five minutes: every transfer longer than that was cut off mid-body as "error decoding response body" and retried. A transcode ignores `Range`, so each retry restarted from byte zero, met the same deadline, and after three attempts the download failed — no feature film at transcode speed ever completed on a device whose audio must be re-encoded, and a large direct copy limped through in five-minute slices with a backoff between each. A read timeout resets on every chunk, so it still catches a dead connection without capping how long a healthy transfer may run | Downloads | UR-071 | Done |
 | DR-290 | A download whose response states no length still reports progress against a predicted total. A transcode is produced as it is sent — chunked, no `Content-Length` — and the worker reported `progress: 0.0` for its whole duration: an empty bar reading "0%" while the byte count climbed for an hour, which is the case every film whose audio must be re-encoded lands in. The backend already fetches the item to decide the audio policy, and that item carries what a prediction needs: the source's size (an `original` download copies the picture, so the output is the source give or take the audio track — and exactly the source when nothing is re-encoded) and its runtime (a preset re-encodes at fixed rates, so the size is rate × runtime, from the same preset table the URL is built from so the two cannot drift). The prediction is made where the URL is resolved and persisted as the row's `file_size`; the worker uses it **only** when the response has no length, the server's figure always wins, an estimated bar is capped at 99% so a low prediction never shows a finished download still running, and the `Completed` event carries the bytes actually written so neither side persists the prediction as the real size. With no prediction the bar is indeterminate, which is honest and was the status quo. The single-video button joins the series/season buttons on the enqueue path so all three resolve — and predict — in one place | Downloads | UR-071 | Done |
+| DR-291 | The offline banner stays off the full-screen player. Every other shell rule in `layoutShell.ts` already treats `/player/*` as immersive; the amber "You're offline" strip was the one piece of chrome still rendered above it. On the native Android video path that is not cosmetic: VideoPlayer makes itself transparent so the ExoPlayer SurfaceView behind the WebView is visible (DR-185), so a shell child that still paints shows *through* the picture as a stripe across the top of the film. Offline is also precisely when a downloaded video plays, so the banner appeared when it was most in the way, and it offers the viewer nothing to act on — local playback needs no server. The rule moves into the pure module as `showOfflineBanner({ pathname, isAuthenticated, isConnected })` rather than staying an inline `{#if}` in the shell, so the immersive-route contract is stated in one tested place | UI | UR-003, UR-043 | Done |

 ---

@@ -503,7 +504,7 @@ Internal architecture, components, and application logic.
 |----------|-------------------------|-------------------------|
 | UR-001 | IR-001, IR-002 | - |
 | UR-002 | IR-013 | DR-003, DR-012, DR-013, DR-014 |
-| UR-003 | IR-003, IR-004, IR-011 | DR-002, DR-004, DR-010, DR-182, DR-183, DR-184, DR-185, DR-186, DR-187, DR-188, DR-190, DR-191, DR-192, DR-193, DR-194, DR-195, DR-196 |
+| UR-003 | IR-003, IR-004, IR-011 | DR-002, DR-004, DR-010, DR-182, DR-183, DR-184, DR-185, DR-186, DR-187, DR-188, DR-190, DR-191, DR-192, DR-193, DR-194, DR-195, DR-196, DR-291 |
 | UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006, DR-129, DR-171, DR-176, DR-177, DR-181, DR-182, DR-183, DR-185, DR-188, DR-203, DR-265 |
 | UR-005 | - | DR-001, DR-005, DR-009, DR-178, DR-179, DR-186, DR-193, DR-195 |
 | UR-006 | IR-005, IR-006, IR-007, IR-008 | DR-200, DR-201 |
@@ -543,7 +544,7 @@ Internal architecture, components, and application logic.
 | UR-040 | IR-025 | DR-051, DR-052, DR-129, DR-130, DR-159, DR-178, DR-179, DR-180, DR-183, DR-190, DR-196, DR-201, DR-203, DR-263, DR-266 |
 | UR-041 | IR-026 | DR-053, DR-160, DR-161, DR-172, DR-182, DR-183, DR-184, DR-185, DR-188, DR-265, DR-266 |
 | UR-042 | IR-009, IR-014 | DR-054 |
-| UR-043 | IR-027 | DR-055 |
+| UR-043 | IR-027 | DR-055, DR-291 |
 | UR-044 | - | DR-056 |
 | UR-045 | - | DR-057 |
 | UR-046 | IR-028 | DR-058 |
@@ -842,6 +843,7 @@ Internal architecture, components, and application logic.
 | UT-252 | A predicted total fills in only when the server sent no length, the server's length always wins, an estimated fraction is capped below 1.0, and the prediction is rate × runtime for a preset and the source's size for `original` | DR-290 | Done |
 | UT-253 | Resolving a queued video row persists its predicted size, and resolving an audio row (no prediction) leaves a size the row already holds untouched | DR-290 | Done |
 | UT-254 | The progress row renders an unknown total as indeterminate rather than "0%", an estimated total as "~N%", an exact one plainly; the store carries the estimate flag through progress and persists the worker's byte count, never the prediction, on completion | DR-290 | Done |
+| UT-255 | The offline banner shows while offline on ordinary routes and never on `/player/*`, and stays off while connected or signed out | DR-291 | Done |
 ### Integration Tests

 | Test ID | Test Description | Traces To | Status |
@@ -23,6 +23,7 @@ import {
  routeOwnsLayout,
  showBottomUi,
  shellReservesBottomInset,
+  showOfflineBanner,
 } from "./layoutShell";

 const authed = (pathname: string) => ({ pathname, isAuthenticated: true });
@@ -236,3 +237,31 @@ describe("profile picker chrome", () => {
    expect(shellReservesBottomInset({ pathname, isAuthenticated: true })).toBe(true);
  });
 });
+
+/**
+ * The offline banner is shell chrome, and the full-screen player is chrome-free
+ * like every other immersive route in this module. On the native Android video
+ * path VideoPlayer makes itself transparent so the ExoPlayer SurfaceView behind
+ * the WebView shows through (DR-185), which means any shell element that still
+ * paints — the amber banner — appears as a stripe across the top of the film.
+ *
+ * TRACES: UR-003 | DR-291 | UT-255
+ */
+describe("showOfflineBanner", () => {
+  it("shows while offline on ordinary routes", () => {
+    expect(showOfflineBanner({ ...authed("/"), isConnected: false })).toBe(true);
+    expect(showOfflineBanner({ ...authed("/library/movies"), isConnected: false })).toBe(true);
+    expect(showOfflineBanner({ ...authed("/downloads"), isConnected: false })).toBe(true);
+  });
+
+  it("stays off the full-screen player, where it would paint over the video", () => {
+    expect(showOfflineBanner({ ...authed("/player/abc123"), isConnected: false })).toBe(false);
+  });
+
+  it("stays off while connected, and while signed out", () => {
+    expect(showOfflineBanner({ ...authed("/"), isConnected: true })).toBe(false);
+    expect(showOfflineBanner({ pathname: "/", isAuthenticated: false, isConnected: false })).toBe(
+      false,
+    );
+  });
+});
@@ -129,3 +129,26 @@ export function showBottomUi(input: BottomUiVisibilityInput): boolean {
 export function shellReservesBottomInset(input: BottomUiVisibilityInput): boolean {
  return !showBottomUi(input);
 }
+
+/**
+ * Whether the shell renders the offline banner ("You're offline…").
+ *
+ * The banner is shell chrome, and it is the last piece of it that still
+ * rendered over the full-screen player — every other rule in this module
+ * already treats `/player/*` as immersive. On the native Android video path
+ * that is not merely untidy: VideoPlayer makes itself transparent so the
+ * ExoPlayer SurfaceView behind the WebView can be seen (DR-185), so any shell
+ * element that still paints shows through the film. Offline is also exactly
+ * when a downloaded video plays, so the banner was most likely to be there
+ * precisely when it was most in the way — and it says nothing the viewer can
+ * act on while watching: local playback needs no server.
+ *
+ * TRACES: UR-003, UR-043 | DR-291
+ */
+export function showOfflineBanner({
+  pathname,
+  isAuthenticated,
+  isConnected,
+}: BottomUiVisibilityInput & { isConnected: boolean }): boolean {
+  return isAuthenticated && !isConnected && !pathname.startsWith("/player/");
+}
@@ -39,6 +39,7 @@
    showGlobalHeader as computeShowGlobalHeader,
    routeOwnsLayout as computeRouteOwnsLayout,
    shellReservesBottomInset,
+    showOfflineBanner as computeShowOfflineBanner,
  } from "$lib/utils/layoutShell";
  import { registerNavigationTracking } from "$lib/utils/navigation";
  import { useScrollRestore } from "$lib/utils/scrollContainer";
@@ -108,6 +109,17 @@
    shellReservesBottomInset({ pathname, isAuthenticated: $isAuthenticated }),
  );

+  // The offline banner is shell chrome like the rest, so it stays off the
+  // full-screen player — where on the native video path it would paint a
+  // stripe straight through the film (DR-291).
+  const offlineBannerVisible = $derived(
+    computeShowOfflineBanner({
+      pathname,
+      isAuthenticated: $isAuthenticated,
+      isConnected: $isConnected,
+    }),
+  );
+
  onMount(async () => {
    // Detect platform first (synchronously, before any await) so the global
    // mini player's Android visibility gate is correct from the first render.
@@ -283,8 +295,8 @@
  style:padding-bottom={shellPadsBottom ? "var(--safe-bottom)" : undefined}
 >
  {#if isInitialized}
-    <!-- Offline indicator banner -->
-    {#if $isAuthenticated && !$isConnected}
+    <!-- Offline indicator banner (never over the player — DR-291) -->
+    {#if offlineBannerVisible}
      <div
        class="bg-amber-600/90 text-white px-4 py-2 text-sm flex items-center justify-center gap-2 shrink-0"
      >