From dd45af20ef75ce7cbe605a2a3f3e70972f453055 Mon Sep 17 00:00:00 2001
From: Ilia Denisov <evilcart@gmail.com>
Date: Tue, 16 Jun 2026 07:18:50 +0200
Subject: [PATCH] fix(ads): fade the scroll rewind + keep the banner strip
 height constant
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two polish fixes (owner feedback):

- Scroll loop: a long message that scrolled to its right edge rewound with a hard
  jump (no fade). It now runs the same fade as a message change at each rewind:
  fade out at the edge, reset the scroll while hidden, fade the same message back
  in, then scroll again.
- Strip height: during the fade gap the message layer is removed, which let the
  strip collapse by ~1-2px. An always-present invisible spacer now reserves one
  line of height and the message is overlaid absolutely, so the strip height is
  constant whether or not the message is showing.

Verified live: opacity sampling shows a full fade-out → gap → fade-in at each
scroll rewind (~every 6s), and the .ad height stays a single constant value
(30.31px) across the whole cycle including the gap. Loop-fade unit-tested.
---
 ui/src/components/AdBanner.svelte | 20 +++++++++++++++++---
 ui/src/lib/banner.test.ts         | 15 +++++++++++++++
 ui/src/lib/banner.ts              | 10 ++++++++--
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/ui/src/components/AdBanner.svelte b/ui/src/components/AdBanner.svelte
index 66e878f..2a527fa 100644
--- a/ui/src/components/AdBanner.svelte
+++ b/ui/src/components/AdBanner.svelte
@@ -116,6 +116,10 @@
 <!-- svelte-ignore a11y_no_static_element_interactions -->
 <!-- svelte-ignore a11y_click_events_have_key_events -->
 <div class="ad" bind:this={viewport} onclick={onExternalLinkClick}>
+  <!-- An always-present, invisible spacer reserves exactly one line of height, so the strip never
+       collapses while the message layer is absent during the fade gap (the message is overlaid
+       absolutely, so its presence/absence does not change the strip height). -->
+  <span class="reserve" aria-hidden="true">&nbsp;</span>
   {#if visible}
     <div class="fadewrap" in:inFade out:fade={{ duration: eff.fadeOutMs }}>
       <div
@@ -131,6 +135,7 @@
 
 <style>
   .ad {
+    position: relative;
     overflow: hidden;
     white-space: nowrap;
     padding: 6px 0;
@@ -142,9 +147,18 @@
     border-top: 1px solid var(--border);
     border-bottom: 1px solid var(--border);
     user-select: none;
-    /* A stable height so the strip does not collapse during the fade gap (the message layer is
-       removed between fade-out and fade-in). */
-    min-height: calc(0.85rem * 1.2 + 12px);
+  }
+  /* Reserves one line of height inside the padding so .ad keeps a constant height even during the
+     fade gap; invisible and non-interactive. */
+  .reserve {
+    visibility: hidden;
+  }
+  /* The message layer is overlaid on the reserved line, so showing/hiding it never resizes .ad. */
+  .fadewrap {
+    position: absolute;
+    top: 6px;
+    left: 0;
+    right: 0;
   }
   .track {
     display: inline-block;
diff --git a/ui/src/lib/banner.test.ts b/ui/src/lib/banner.test.ts
index 88e4aa3..78c0afa 100644
--- a/ui/src/lib/banner.test.ts
+++ b/ui/src/lib/banner.test.ts
@@ -135,6 +135,21 @@ describe('createBannerRotator', () => {
     r.stop();
   });
 
+  it('fades the scroll loop of a long message (out, rewind, in) instead of a hard reset', () => {
+    vi.useFakeTimers();
+    const c: BannerTimings = { holdMs: 30000, edgePauseMs: 100, scrollPxPerSec: 1000, fadeOutMs: 100, gapMs: 50, fadeInMs: 100 };
+    const { host, calls } = recordingHost(200); // overflows -> scroll dur = 200ms
+    const r = createBannerRotator([{ weight: 1, messages: ['long'] }], host, c);
+    r.start();
+    // fadeIn -> measure -> edgePause -> scrollTo -> (dur + edgePause) -> loop boundary
+    vi.advanceTimersByTime(c.fadeInMs + c.edgePauseMs + 200 + c.edgePauseMs + 5);
+    expect(calls).toContain('scroll');
+    expect(calls).toContain('hide'); // the loop fades out at the right edge (not a hard reset)
+    vi.advanceTimersByTime(c.fadeOutMs + c.gapMs + 5); // -> the same message fades back in
+    expect(calls.filter((x) => x === 'show').length).toBeGreaterThanOrEqual(2);
+    r.stop();
+  });
+
   it('swaps instantly when fade timings are zero (reduce-motion)', () => {
     vi.useFakeTimers();
     const zero: BannerTimings = { ...cfg, fadeOutMs: 0, gapMs: 0, fadeInMs: 0 };
diff --git a/ui/src/lib/banner.ts b/ui/src/lib/banner.ts
index 03aa349..3a09ac8 100644
--- a/ui/src/lib/banner.ts
+++ b/ui/src/lib/banner.ts
@@ -138,8 +138,14 @@ export function createBannerRotator(
         if (Date.now() - cycleStart >= config.holdMs) {
           advance();
         } else {
-          host.resetScroll();
-          scrollCycle(over);
+          // Loop the same message with a fade: fade out at the right edge, rewind to the start
+          // while hidden, fade the same message back in, then scroll again — so the rewind gets
+          // the same fade as a message change, not a hard jump.
+          host.hide(config.fadeOutMs);
+          at(config.fadeOutMs + config.gapMs, () => {
+            host.show(lastShown);
+            at(config.fadeInMs, () => scrollCycle(over));
+          });
         }
       });
     });