diff --git a/frontend/e2e/navigation-no-fall.spec.ts b/frontend/e2e/navigation-no-fall.spec.ts new file mode 100644 index 0000000..6e2bef4 --- /dev/null +++ b/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); + }); +}); diff --git a/frontend/src/app/components/modal/block-edit.component.ts b/frontend/src/app/components/modal/block-edit.component.ts index 371f299..f54ee87 100644 --- a/frontend/src/app/components/modal/block-edit.component.ts +++ b/frontend/src/app/components/modal/block-edit.component.ts @@ -140,6 +140,7 @@ export function createDoneValue(defaultDone: boolean, currentDone: boolean, edit
+
} @@ -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 { diff --git a/frontend/src/app/components/page/page.component.scss b/frontend/src/app/components/page/page.component.scss index 4630d62..f6b61cd 100644 --- a/frontend/src/app/components/page/page.component.scss +++ b/frontend/src/app/components/page/page.component.scss @@ -116,7 +116,11 @@ } } - .confirm-delete { + // Projected into , which hoists itself to (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 { diff --git a/frontend/src/app/components/pages/pages.component.html b/frontend/src/app/components/pages/pages.component.html index 00d59b3..38e86d3 100644 --- a/frontend/src/app/components/pages/pages.component.html +++ b/frontend/src/app/components/pages/pages.component.html @@ -10,13 +10,16 @@
- @if (selectedPage(); as page) { + + @for (page of selectedPageList(); track page.id) { - } @else { + } @empty {

Add a new page to get started!

}
diff --git a/frontend/src/app/components/pages/pages.component.scss b/frontend/src/app/components/pages/pages.component.scss index 9d5f9c6..68e6f4b 100644 --- a/frontend/src/app/components/pages/pages.component.scss +++ b/frontend/src/app/components/pages/pages.component.scss @@ -46,7 +46,11 @@ } } - .confirm-delete { + // Projected into , which hoists itself to (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 { diff --git a/frontend/src/app/components/pages/pages.component.ts b/frontend/src/app/components/pages/pages.component.ts index 46e01b7..044e722 100644 --- a/frontend/src/app/components/pages/pages.component.ts +++ b/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(() => { + const page = this.selectedPage(); + return page ? [page] : []; + }); + readonly confirmDeletePageName = computed(() => { const id = this.confirmDeletePageId(); if (!id) return ''; diff --git a/frontend/src/app/components/tasks/tasks.component.ts b/frontend/src/app/components/tasks/tasks.component.ts index 4a11fd0..c0f7a42 100644 --- a/frontend/src/app/components/tasks/tasks.component.ts +++ b/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); } diff --git a/frontend/src/app/components/tower/tower.component.ts b/frontend/src/app/components/tower/tower.component.ts index 1053bf3..d75488f 100644 --- a/frontend/src/app/components/tower/tower.component.ts +++ b/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[] { - 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(null); + private readonly injector = inject(Injector); private readonly stackZone = viewChild>('stackZone'); private readonly towerRoot = viewChild>('towerRoot'); private readonly maxVisibleBlocks = signal(null); private resizeObserver: ResizeObserver | null = null; - private destroyed = false; - private readonly animationFrames = new Set(); // ── 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(); + /** 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(); /** 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(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(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 { diff --git a/frontend/src/app/components/tower/tower.component.vitest.ts b/frontend/src/app/components/tower/tower.component.vitest.ts index dacf111..58f1292 100644 --- a/frontend/src/app/components/tower/tower.component.vitest.ts +++ b/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([]);