Fix visual bugs

frontend/e2e/navigation-no-fall.spec.ts

@@ -0,0 +1,108 @@
+import { test, expect } from '@playwright/test';
+
+/**
+ * Regression: navigating between pages must NOT replay the falling animation.
+ *
+ * The bug: `lt-page` (and its date-range slider) were reused across navigation,
+ * so the previous page's stale `dateRange` was applied to the destination page's
+ * towers on their first render. When the destination's blocks are NEWER than the
+ * stale range they render off-screen (ascending) and then visibly "fall" in a
+ * frame later when the slider corrects the range — i.e. all blocks fall at once.
+ * The fix rebuilds the `lt-page` subtree on page-id change (fresh slider/filter),
+ * matching a reload. This guards the destination ("Newer") page against falling.
+ *
+ * We detect a fall directly: hook the WAAPI `el.animate` used by `playFall` and
+ * the `.descend`/`.ascend` CSS `transitionrun`, both on `lt-block` hosts. These
+ * fire at the JS layer regardless of headless rendering, so the check is stable.
+ *
+ * The two pages are seeded into the offline cache with DISJOINT date ranges
+ * (destination strictly newer) — the precise condition that triggered the bug.
+ */
+test.describe('navigation does not replay the falling animation', () => {
+  test('navigating to a newer-dated page keeps blocks at rest', async ({ page }) => {
+    const nowSec = Math.floor(Date.now() / 1000);
+    const HOUR = 3600;
+    const DAY = 86400;
+
+    await page.addInitScript(
+      ({ nowSec, HOUR, DAY }) => {
+        const uuid = () => (crypto as Crypto).randomUUID();
+        // baseAgeDays/spacingHrs place each page's done blocks in its own window.
+        const tower = (name: string, h: number, baseAgeDays: number) => ({
+          id: uuid(),
+          name,
+          base_color: { h, s: 0.7, l: 0.55 },
+          blocks: [
+            { id: uuid(), tag: 't', description: 'pending', is_done: false, difficulty: 2, created_at: nowSec },
+            ...Array.from({ length: 6 }, (_unused, i) => ({
+              id: uuid(),
+              tag: 't',
+              description: `done ${i}`,
+              is_done: true,
+              difficulty: 1 + (i % 3),
+              created_at: nowSec - baseAgeDays * DAY - (i + 1) * 6 * HOUR,
+            })),
+          ],
+        });
+        const mkPage = (name: string, h: number, baseAgeDays: number) => ({
+          id: uuid(),
+          name,
+          hide_create_tower_button: false,
+          keep_tasks_open: false,
+          default_date_from: null,
+          default_date_to: null,
+          towers: [tower(name + ' A', h, baseAgeDays), tower(name + ' B', h + 0.2, baseAgeDays)],
+        });
+        // Page 0 ("Older") is the post-reload default; "Newer" is strictly more recent.
+        const tree = { pages: [mkPage('Older', 0.05, 6), mkPage('Newer', 0.55, 1)] };
+        const token = uuid();
+        localStorage.setItem('life-towers.token.v4', token);
+        localStorage.setItem('life-towers.cache.v4.' + token, JSON.stringify(tree));
+
+        // Fall detectors on lt-block hosts (WAAPI playFall + CSS descend/ascend).
+        (window as unknown as { __falls: number }).__falls = 0;
+        const isBlock = (el: EventTarget | null) =>
+          el instanceof Element && el.tagName === 'LT-BLOCK';
+        const origAnimate = Element.prototype.animate;
+        Element.prototype.animate = function (kf: unknown, opts?: unknown) {
+          try {
+            const s = JSON.stringify(kf ?? '');
+            if (isBlock(this) && (s.includes('translateY') || s.includes('transform')))
+              (window as unknown as { __falls: number }).__falls++;
+          } catch {
+            /* ignore */
+          }
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          return origAnimate.call(this, kf as any, opts as any);
+        };
+        document.addEventListener(
+          'transitionrun',
+          (e) => {
+            if ((e as TransitionEvent).propertyName === 'transform' && isBlock(e.target))
+              (window as unknown as { __falls: number }).__falls++;
+          },
+          true,
+        );
+      },
+      { nowSec, HOUR, DAY },
+    );
+
+    await page.goto('/');
+    // Lands on "Older"; wait for its stack to render and settle.
+    await page.waitForSelector('lt-block', { timeout: 15000 });
+    await page.waitForTimeout(2000);
+
+    // Reset the detector right before navigation, then switch to "Newer".
+    await page.evaluate(() => ((window as unknown as { __falls: number }).__falls = 0));
+    await page.locator('lt-select-add .top').first().click();
+    await page.locator('lt-select-add .option', { hasText: 'Newer' }).first().click();
+
+    // Give any fall a full beat to fire (the animation is 1.5s).
+    await page.waitForTimeout(1800);
+
+    const falls = await page.evaluate(() => (window as unknown as { __falls: number }).__falls);
+    expect(falls, 'block fall animations fired on navigation').toBe(0);
+    // Sanity: the destination page actually rendered its blocks.
+    expect(await page.locator('lt-block').count()).toBeGreaterThan(0);
+  });
+});

frontend/src/app/components/modal/block-edit.component.ts

@@ -140,6 +140,7 @@ export function createDoneValue(defaultDone: boolean, currentDone: boolean, edit
 
           <div class="bottom">
             <button (click)="onDelete(b.id); $event.stopPropagation()">Delete</button>
+            <button (click)="saveAndExit(b.id); $event.stopPropagation()">Save and exit</button>
           </div>
         </div>
       }
@@ -550,21 +551,27 @@ export function createDoneValue(defaultDone: boolean, currentDone: boolean, edit
       }
 
       .bottom {
-        height: 32px;
+        min-height: 32px;
         @media (max-width: $mobile-width) {
-          height: 24px;
+          min-height: 24px;
         }
-        position: relative;
+        display: flex;
+        align-items: center;
+        // Existing-block cards carry two buttons (Delete + Save and exit) —
+        // push them to opposite edges of the card. The create card has a single
+        // button, re-centered below.
+        justify-content: space-between;
+        gap: var(--medium-padding);
 
         button {
           margin: 0;
-          position: absolute;
-          left: 50%;
-          top: 50%;
-          transform: translateY(-50%) translateX(-50%);
         }
       }
 
+      &.create-card .bottom {
+        justify-content: center;
+      }
+
       @media (max-width: $mobile-width) {
         lt-select-add,
         .done-checkbox {
@@ -574,6 +581,7 @@ export function createDoneValue(defaultDone: boolean, currentDone: boolean, edit
 
         .bottom {
           min-height: 42px;
+          gap: var(--medium-padding);
 
           button {
             width: max-content;
@@ -783,6 +791,16 @@ export class BlockEditComponent implements AfterViewInit {
     this.delete.emit(id);
   }
 
+  /**
+   * Flush any pending edits for the card (notably the description, which is
+   * otherwise only saved on blur) and close the carousel — mirrors the create
+   * card's "Create and exit" affordance for the existing-block cards.
+   */
+  saveAndExit(id: string): void {
+    this.flushExisting(id);
+    this.close.emit();
+  }
+
   // ── Create-card mutations ──────────────────────────────────────────────────
 
   updateNewTag(tag: string): void {

frontend/src/app/components/page/page.component.scss

@@ -116,7 +116,11 @@
     }
   }
 
-  .confirm-delete {
+  // Projected into <lt-modal>, which hoists itself to <body> (see
+  // modal.component) — that moves this content out of :host, so a :host-scoped
+  // selector no longer matches. @at-root drops the :host prefix; the
+  // [_ngcontent] encapsulation attribute still scopes it to this component.
+  @at-root .confirm-delete {
     @include card();
     width: 66vw;
     max-width: 500px;
@@ -152,7 +156,7 @@
 
     .confirm-buttons {
       display: flex;
-      justify-content: center;
+      justify-content: space-between;
       gap: var(--large-padding);
 
       button {

frontend/src/app/components/pages/pages.component.html

@@ -10,13 +10,16 @@
 </div>
 
 <div class="page-container">
-  @if (selectedPage(); as page) {
+  <!-- 0-or-1 element @for keyed by page.id: rebuilds the page (fresh date-range
+       slider/filter) on navigation, reuses it on same-page edits. See
+       PagesComponent.selectedPageList. -->
+  @for (page of selectedPageList(); track page.id) {
     <lt-page
       [page]="page"
       [animateInitialStack]="page.id === animateInitialStackPageId()"
       (dragHappening)="dragHappening.set($event)"
     />
-  } @else {
+  } @empty {
     <p>Add a new page to get started!</p>
   }
 </div>

frontend/src/app/components/pages/pages.component.scss

@@ -46,7 +46,11 @@
     }
   }
 
-  .confirm-delete {
+  // Projected into <lt-modal>, which hoists itself to <body> (see
+  // modal.component) — that moves this content out of :host, so a :host-scoped
+  // selector no longer matches. @at-root drops the :host prefix; the
+  // [_ngcontent] encapsulation attribute still scopes it to this component.
+  @at-root .confirm-delete {
     @include card();
     width: 66vw;
     max-width: 500px;
@@ -76,7 +80,7 @@
 
     .confirm-buttons {
       display: flex;
-      justify-content: center;
+      justify-content: space-between;
       gap: var(--large-padding);
 
       button {

frontend/src/app/components/pages/pages.component.ts

@@ -83,6 +83,24 @@ export class PagesComponent implements OnDestroy {
     return pages[0] ?? null;
   });
 
+  /**
+   * The selected page as a 0-or-1 element list, so the template's
+   * `@for (… track page.id)` REBUILDS the `lt-page` subtree when the page *id*
+   * changes (navigation) but REUSES it on same-page edits (id unchanged).
+   *
+   * Recreating on navigation gives each page a fresh date-range slider + filter,
+   * exactly like a page reload. Without it, `lt-page` (and its slider) are reused
+   * across pages, so the previous page's stale `dateRange` is applied to the new
+   * page's towers on their very first render. When the new page's blocks fall
+   * outside that stale range they render out-of-range (ascending, off-screen) and
+   * then visibly "fall" into place a frame later when the slider corrects the
+   * range — the bug this guards against.
+   */
+  readonly selectedPageList = computed<Page[]>(() => {
+    const page = this.selectedPage();
+    return page ? [page] : [];
+  });
+
   readonly confirmDeletePageName = computed(() => {
     const id = this.confirmDeletePageId();
     if (!id) return '';

frontend/src/app/components/tasks/tasks.component.ts

@@ -208,12 +208,22 @@ export function taskListMaxHeight(expanded: boolean): string {
                 transition: opacity $short-animation-time, transform $short-animation-time;
               }
 
-              &:hover,
+              // Reveal on hover only on real hover-capable pointers. On touch,
+              // :hover sticks to whatever ends up under the finger after the
+              // tapped task is removed — the next task slides up and would show
+              // its ✓. Keyboard focus + the genuine press still reveal it.
               &:focus-visible {
                 box-shadow: $shadow;
                 transform: scale(1.05);
                 &::after { opacity: 0.85; }
               }
+              @media (hover: hover) and (pointer: fine) {
+                &:hover {
+                  box-shadow: $shadow;
+                  transform: scale(1.05);
+                  &::after { opacity: 0.85; }
+                }
+              }
               &:active {
                 transform: scale(0.95);
                 &::after { opacity: 1; transform: translateY(1px) scale(1.05); }

frontend/src/app/components/tower/tower.component.ts

@@ -7,6 +7,9 @@ import {
   computed,
   effect,
   untracked,
+  inject,
+  afterNextRender,
+  Injector,
   AfterViewInit,
   OnDestroy,
   ElementRef,
@@ -128,25 +131,35 @@ export function selectVisibleStyledBlocks(
  * The two guarantees the user asked for live here:
  *   - **No fall on page load.** The very first render of a tower's stack never
  *     animates (`firstRender` ⇒ []), except the deliberate example showcase.
- *   - **Always fall on add / tick.** After that first render, any block whose id
- *     is genuinely new this round (a ticked task, an added done block, or a
- *     remote add picked up over SSE) and is currently resting & visible falls.
+ *   - **Always fall on add / tick.** After that first render, any block that has
+ *     arrived but not yet fallen (`pendingFallIds` — a ticked task, an added
+ *     done block, or a remote add picked up over SSE) falls once it is resting
+ *     & visible.
  *
- * Date-range reshuffles never appear here: a block re-entering range keeps its
- * id (so it isn't "new"), and blocks flying out aren't resting (`restingVisibleIds`
- * only holds opacity-1 blocks).
+ * `pendingFallIds` is an ACCUMULATOR, not a single-round "new this round" diff:
+ * an arrival that can't fall yet (e.g. still out of the date range on the
+ * reconcile that first sees it) stays pending and falls on the later reconcile
+ * that brings it to rest. This is what makes ticking robust to the two-pass
+ * reconcile a tick triggers when the slider is live — the block change runs one
+ * pass with the stale range, then the slider's snap re-emits a wider range and
+ * runs a second. A per-round diff would consume the arrival in the first pass
+ * and lose the fall in that gap.
+ *
+ * Date-range *reshuffles* of already-fallen blocks never appear here: a block
+ * re-entering range keeps its id (so it isn't a fresh arrival), and blocks
+ * flying out aren't resting (`restingVisibleIds` only holds opacity-1 blocks).
  */
 export function decideFalls(opts: {
   firstRender: boolean;
   animateInitialStack: boolean;
-  newIds: readonly string[];
+  pendingFallIds: readonly string[];
   restingVisibleIds: ReadonlySet<string>;
 }): string[] {
-  const { firstRender, animateInitialStack, newIds, restingVisibleIds } = opts;
+  const { firstRender, animateInitialStack, pendingFallIds, restingVisibleIds } = opts;
   if (firstRender) {
     return animateInitialStack ? [...restingVisibleIds] : [];
   }
-  return newIds.filter((id) => restingVisibleIds.has(id));
+  return pendingFallIds.filter((id) => restingVisibleIds.has(id));
 }
 
 @Component({
@@ -585,12 +598,11 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
   readonly hiddenBlockCount = signal(0);
   readonly hoveredBlockId = signal<string | null>(null);
 
+  private readonly injector = inject(Injector);
   private readonly stackZone = viewChild<ElementRef<HTMLElement>>('stackZone');
   private readonly towerRoot = viewChild<ElementRef<HTMLElement>>('towerRoot');
   private readonly maxVisibleBlocks = signal<number | null>(null);
   private resizeObserver: ResizeObserver | null = null;
-  private destroyed = false;
-  private readonly animationFrames = new Set<number>();
 
   // ── Derived ────────────────────────────────────────────────────────────────
   /** Pending (not-done) blocks — fed to the tasks accordion. */
@@ -663,6 +675,11 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
 
   /** Done-block ids present at the previous reconcile — diffed to find arrivals. */
   private prevDoneIds = new Set<string>();
+  /** Ids that have arrived (ticked / added / synced) but haven't fallen yet.
+   *  An arrival lingers here until a reconcile renders it resting & visible —
+   *  surviving the extra reconcile a tick triggers via the slider's range snap,
+   *  which would otherwise consume its "newness" before it could fall. */
+  private pendingFallIds = new Set<string>();
   /** False until the stack has been rendered once; gates "no fall on load". */
   private hasRenderedStack = false;
   /** WAAPI fall animations in flight; cancelled on destroy. */
@@ -697,9 +714,6 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
   }
 
   ngOnDestroy(): void {
-    this.destroyed = true;
-    for (const id of this.animationFrames) cancelAnimationFrame(id);
-    this.animationFrames.clear();
     for (const anim of this.runningAnimations) anim.cancel();
     this.runningAnimations.clear();
     this.resizeObserver?.disconnect();
@@ -759,6 +773,23 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
     const firstRender = !this.hasRenderedStack;
     const newIds = ids.filter((id) => !this.prevDoneIds.has(id));
 
+    // Maintain the pending-fall accumulator. On the first render the whole
+    // initial stack is suppressed (no load-fall), so nothing is pending.
+    // Afterwards every fresh arrival becomes pending and STAYS pending until a
+    // reconcile renders it at rest — so the extra reconcile a tick triggers
+    // (the block change runs once with the stale range, then the slider's range
+    // snap re-emits and runs a second) can't consume its "newness" before it
+    // ever rests & falls. Forget any pending id that's no longer done (deleted
+    // or un-ticked before it fell).
+    if (firstRender) {
+      this.pendingFallIds.clear();
+    } else {
+      for (const id of newIds) this.pendingFallIds.add(id);
+    }
+    for (const id of [...this.pendingFallIds]) {
+      if (!idSet.has(id)) this.pendingFallIds.delete(id);
+    }
+
     // Build the styled list: in-range blocks rest at the bottom; blocks past the
     // upper date bound fly up and out (declarative `.ascend` transition); blocks
     // below the lower bound drop out of the list instantly. In-range blocks carry
@@ -786,9 +817,13 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
       return;
     }
 
-    // Reserve a visible slot for the newest just-added block so a capped stack
-    // still shows (and can fall) it.
-    const reserveId = !firstRender && newIds.length > 0 ? newIds[newIds.length - 1] : null;
+    // Reserve a visible slot for the newest still-pending arrival so a capped
+    // stack still shows (and can fall) it. Drawn from the accumulator, not just
+    // this round's new ids, so the slot survives the second reconcile of a tick.
+    const reserveId =
+      !firstRender && this.pendingFallIds.size > 0
+        ? (ids.filter((id) => this.pendingFallIds.has(id)).pop() ?? null)
+        : null;
     // Captured before the `_visibleBlocks.set` below — lets the cap keep
     // currently-shown blocks that are now flying out so their exit animates.
     const prevVisibleIds = new Set(this._visibleBlocks().map((b) => b.id));
@@ -805,7 +840,14 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
       visibleStyled.filter((b) => b._opacity === '1').map((b) => b.id),
     );
 
-    const toFall = decideFalls({ firstRender, animateInitialStack, newIds, restingVisibleIds });
+    const toFall = decideFalls({
+      firstRender,
+      animateInitialStack,
+      pendingFallIds: [...this.pendingFallIds],
+      restingVisibleIds,
+    });
+    // These are falling now — they're no longer awaiting a resting render.
+    for (const id of toFall) this.pendingFallIds.delete(id);
 
     this.prevDoneIds = idSet;
     this.hasRenderedStack = true;
@@ -834,30 +876,34 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
 
   /**
    * Play the gravity "fall" on the given blocks' square elements via the Web
-   * Animations API, after they've rendered at their resting position. WAAPI
-   * animates the elements in from above on the next frame regardless of
-   * change-detection timing, so the fall always fires (the old approach flipped
-   * a CSS class across two rAFs and would intermittently drop the animation).
+   * Animations API.
+   *
+   * Scheduled with `afterNextRender`, which fires after Angular has committed
+   * the reconcile's DOM but BEFORE the browser paints. That ordering is the
+   * whole point: the square is rendered at its resting position, and the fall
+   * (which starts off-screen via `playFall`'s `fill: 'backwards'`) is installed
+   * in the same frame before paint — so the resting square never flashes for a
+   * frame before falling. A bare `requestAnimationFrame` can instead land a
+   * frame after change detection has already painted the block at rest, which
+   * is exactly the "dropped square appears for a split second, then jumps up and
+   * falls" glitch this avoids. WAAPI is still the right tool for the animation
+   * itself: it's immune to change-detection timing once started.
+   *
+   * Because the hook runs after the reconcile's render, every id in `blockIds`
+   * is in the DOM by now — no retry/poll, which would only mask a broken timing
+   * contract (and reintroduce the flash).
    */
-  private scheduleFall(blockIds: string[], attempt = 0): void {
-    this.requestFrame(() => {
-      if (this.destroyed) return;
-      const zone = this.stackZone()?.nativeElement;
-      if (!zone) return;
-      const pending: string[] = [];
-      for (const id of blockIds) {
-        const selector = `lt-block[data-block-id="${this.cssEscape(id)}"]`;
-        const els = zone.querySelectorAll<HTMLElement>(selector);
-        if (els.length === 0) {
-          // The block hasn't rendered yet (change detection can land a frame
-          // late); retry a couple of frames before giving up.
-          pending.push(id);
-          continue;
+  private scheduleFall(blockIds: string[]): void {
+    afterNextRender(
+      () => {
+        const zone = this.stackZone()!.nativeElement;
+        for (const id of blockIds) {
+          const selector = `lt-block[data-block-id="${this.cssEscape(id)}"]`;
+          zone.querySelectorAll<HTMLElement>(selector).forEach((el) => this.playFall(el));
         }
-        els.forEach((el) => this.playFall(el));
-      }
-      if (pending.length > 0 && attempt < 3) this.scheduleFall(pending, attempt + 1);
-    });
+      },
+      { injector: this.injector },
+    );
   }
 
   private playFall(el: HTMLElement): void {
@@ -888,18 +934,6 @@ export class TowerComponent implements AfterViewInit, OnDestroy {
     return value.replace(/["\\]/g, '\\$&');
   }
 
-  private requestFrame(callback: () => void): void {
-    if (typeof requestAnimationFrame !== 'function') {
-      callback();
-      return;
-    }
-    const id = requestAnimationFrame(() => {
-      this.animationFrames.delete(id);
-      if (!this.destroyed) callback();
-    });
-    this.animationFrames.add(id);
-  }
-
   // ── Event handlers ─────────────────────────────────────────────────────────
 
   onRename(event: Event): void {

frontend/src/app/components/tower/tower.component.vitest.ts

@@ -134,7 +134,7 @@ describe('decideFalls', () => {
       decideFalls({
         firstRender: true,
         animateInitialStack: false,
-        newIds: ['a', 'b', 'c'],
+        pendingFallIds: ['a', 'b', 'c'],
         restingVisibleIds: new Set(['a', 'b', 'c']),
       }),
     ).toEqual([]);
@@ -145,7 +145,7 @@ describe('decideFalls', () => {
       decideFalls({
         firstRender: true,
         animateInitialStack: true,
-        newIds: ['a', 'b'],
+        pendingFallIds: ['a', 'b'],
         restingVisibleIds: new Set(['a', 'b']),
       }),
     ).toEqual(['a', 'b']);
@@ -156,29 +156,47 @@ describe('decideFalls', () => {
       decideFalls({
         firstRender: false,
         animateInitialStack: false,
-        newIds: ['new-1'],
+        pendingFallIds: ['new-1'],
         restingVisibleIds: new Set(['old-1', 'old-2', 'new-1']),
       }),
     ).toEqual(['new-1']);
   });
 
-  it('does not fall a new block that is capped out of view or out of range', () => {
+  it('does not fall a pending block that is capped out of view or out of range', () => {
+    // Out of range / capped out ⇒ not resting-visible ⇒ stays pending, no fall
+    // THIS round. The caller keeps it in the accumulator so it falls later.
     expect(
       decideFalls({
         firstRender: false,
         animateInitialStack: false,
-        newIds: ['new-hidden'],
+        pendingFallIds: ['new-hidden'],
         restingVisibleIds: new Set(['old-1', 'old-2']),
       }),
     ).toEqual([]);
   });
 
-  it('falls nothing when no ids are new (e.g. a date-range reshuffle)', () => {
+  it('falls a still-pending arrival on the LATER reconcile that brings it to rest', () => {
+    // Regression: ticking with the date-slider live runs two reconciles. The
+    // first sees the block out of range (it can't fall — see the case above);
+    // the second, after the slider snaps the range wider, brings it to rest.
+    // Because the id is still pending (not a one-shot "new this round" diff), it
+    // falls now instead of appearing instantly as a static square.
     expect(
       decideFalls({
         firstRender: false,
         animateInitialStack: false,
-        newIds: [],
+        pendingFallIds: ['ticked'],
+        restingVisibleIds: new Set(['old-1', 'ticked']),
+      }),
+    ).toEqual(['ticked']);
+  });
+
+  it('falls nothing when nothing is pending (e.g. a date-range reshuffle of settled blocks)', () => {
+    expect(
+      decideFalls({
+        firstRender: false,
+        animateInitialStack: false,
+        pendingFallIds: [],
         restingVisibleIds: new Set(['old-1', 'old-2', 'old-3']),
       }),
     ).toEqual([]);