From d0017e4a7827df5501f7b2b76018b05a35175fed Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sat, 12 Sep 2026 20:28:42 -0400 Subject: [PATCH] fix(toast): prefer evicting toasts that have been visible >= 1.5s during a burst Fixes #80 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA --- src/app/state/toast.test.ts | 30 +++++++++++++++++++++++++ src/app/state/toast.ts | 44 ++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/app/state/toast.test.ts b/src/app/state/toast.test.ts index b3999ec48..47521f619 100644 --- a/src/app/state/toast.test.ts +++ b/src/app/state/toast.test.ts @@ -127,6 +127,36 @@ test('toastQueueAtom keeps a new transient toast even when the cap is full of st assert.equal(ids.length, 6); }); +// --- #80: eviction prefers toasts that have already been visible a while --- + +test('toastQueueAtom prefers evicting an old-enough toast over a fresh one when both exist', (t) => { + t.mock.timers.enable({ apis: ['Date'] }); + const store = createStore(); + + store.set(toastQueueAtom, makeToast('old')); // t = 0 + t.mock.timers.tick(2000); // 'old' is now well past the 1.5s min-visible mark + store.set(toastQueueAtom, makeToast('mid')); // t = 2000, too young to prefer + for (let i = 0; i < 4; i += 1) store.set(toastQueueAtom, makeToast(`n${i}`)); // fills to cap + 1 + + const ids = store.get(toastQueueAtom).map((toast) => toast.id); + // 'old' has been visible long enough to be preferred for eviction over the + // still-young 'mid', even though 'old' isn't the only non-sticky candidate. + assert.ok(!ids.includes('old')); + assert.ok(ids.includes('mid')); + assert.equal(ids.length, 5); +}); + +test('toastQueueAtom falls back to oldest-first when no toast is old enough yet', () => { + const store = createStore(); + // All appended back-to-back within the same tick — none has reached the + // 1.5s min-visible mark, so eviction falls back to plain oldest-first. + for (let i = 0; i < 7; i += 1) store.set(toastQueueAtom, makeToast(`t${i}`)); + assert.deepEqual( + store.get(toastQueueAtom).map((toast) => toast.id), + ['t2', 't3', 't4', 't5', 't6'], + ); +}); + test('createDownloadToast: filename in body, no room navigation, unique ids', () => { const a = createDownloadToast('photo.jpg'); assert.equal(a.displayName, 'Downloaded'); diff --git a/src/app/state/toast.ts b/src/app/state/toast.ts index 1a3a7f3c9..61da74abc 100644 --- a/src/app/state/toast.ts +++ b/src/app/state/toast.ts @@ -12,6 +12,7 @@ export type ToastNotif = { hashPath?: string; // overrides window.location.hash navigation when set onClick?: () => void; // custom click handler; skips hash navigation when set sticky?: boolean; // when true, does not auto-dismiss — use for action toasts that require a click + createdAt?: number; // set by toastQueueAtom when enqueued; used for #80 eviction ordering }; // Build a "download complete" system toast. Kept folds-free here (the icon src is @@ -50,22 +51,45 @@ const baseAtom = atom([]); // can't stack unbounded and cover the viewport. const MAX_TOASTS = 5; +// #80 — a toast younger than this hasn't had a fair chance to be read yet, so +// eviction prefers dropping an older, already-seen toast over a fresh one when +// a burst arrives while several toasts are still on screen. +const MIN_VISIBLE_MS = 1500; + +// Pick the index (within [0, excludeLastIndex)) of the toast to evict: prefer +// the oldest non-sticky toast that's already been visible at least +// MIN_VISIBLE_MS; if none qualifies yet (e.g. a burst that all arrived within +// the same beat), fall back to the oldest non-sticky toast regardless of age +// so the cap is still enforced. Sticky action toasts are never evicted. +const findEvictionIndex = (toasts: ToastNotif[], excludeLastIndex: number, now: number): number => { + for (let i = 0; i < excludeLastIndex; i += 1) { + const t = toasts[i]; + if (!t.sticky && now - (t.createdAt ?? now) >= MIN_VISIBLE_MS) return i; + } + for (let i = 0; i < excludeLastIndex; i += 1) { + if (!toasts[i].sticky) return i; + } + return -1; +}; + // Write-only setter used in ClientNonUIFeatures export const toastQueueAtom = atom( (get) => get(baseAtom), (get, set, notif) => { if (notif === null) return; // no-op guard + // Stamp in place (not a copy) so the queue keeps holding the exact object + // the caller passed in — callers/tests may rely on reference equality. + notif.createdAt = Date.now(); + const now = notif.createdAt; const next = [...get(baseAtom), notif]; - // Over cap: drop the oldest NON-sticky toasts (transient message/error - // toasts auto-dismiss anyway); never drop a sticky action toast, which - // requires a click. The `length - 1` bound excludes the just-appended - // newest, so a fresh toast is never the one dropped — if everything older - // is sticky the cap simply stretches rather than eating the new notice. - for (let i = 0; i < next.length - 1 && next.length > MAX_TOASTS; i += 1) { - if (!next[i].sticky) { - next.splice(i, 1); - i -= 1; - } + // The last index (the just-appended newest toast) is never a candidate, so + // a fresh toast is never the one dropped — if everything older is sticky + // (or, now, too young) the cap simply stretches rather than eating the new + // notice. + while (next.length > MAX_TOASTS) { + const idx = findEvictionIndex(next, next.length - 1, now); + if (idx === -1) break; + next.splice(idx, 1); } set(baseAtom, next); },