refactor(ui): extract motionSafe() for reduced-motion transition params

[nsollazzo]

Daily simplify (POS-159) — ui repo

Why this one (highest impact-to-risk today): FilterToolbar and Toaster each hand-derived the same reduced-motion transition gate — three verbatim copies of one accessibility contract (reducedMotion.current ? { duration: 0 } : …). This is exactly the third repetition (DRY), and the pattern is subtly error-prone: a partial gate (zeroing duration but leaving a y/x offset) still animates position under prefers-reduced-motion. Centralizing it removes that whole failure mode for this and any future animated component, at near-zero risk.

Before

// FilterToolbar.svelte
const slideParams = $derived(reducedMotion.current ? { duration: 0 } : { duration: 140 });
// Toaster.svelte
const inParams  = $derived(reducedMotion.current ? { duration: 0 } : { y: -8, duration: 300 });
const outParams = $derived(reducedMotion.current ? { duration: 0 } : { duration: 300 });

After

// new: src/lib/actions/motion.ts
export function motionSafe<T extends { duration?: number }>(params: T): T | { duration: 0 } {
	return reducedMotion.current ? { duration: 0 } : params;
}

// call sites
const slideParams = $derived(motionSafe({ duration: 140 }));
const inParams  = $derived(motionSafe({ y: -8, duration: 300 }));
const outParams = $derived(motionSafe({ duration: 300 }));

• Behavior-preserving; still reactive (called inside $derived).
• New helper is a separate module so reducedMotion.ts stays the pure primitive and motionSafe is unit-testable by mocking its one dependency.
• Internal only — not added to the public entry point (index.ts). Exporting it for consumers is a separate product/API call for the catalog/curation owner; flagged, not decided here.

Gates (local, worktree off origin/main)

• pnpm lint ✅ (prettier + eslint)
• pnpm check ✅ (svelte-check: 0 errors / 0 warnings)
• pnpm exec vitest run ✅ — new motion.test.ts (2, both branches) + Toaster & FilterToolbar suites (20) all green
• pnpm check:unused ✅ (knip clean)

Scope is surgical: +56 / −4 across 4 files. Do not merge — handing to R. Cutie for review + e2e.

[nsollazzo]

R. Cutie — review: approve-with-nits ✅ (human merge required)

Reviewed the whole diff + the code it touches (reducedMotion.ts, both call sites, and the surrounding $derived reactivity), traced the reduced-motion path, and re-ran the gates from a clean worktree off origin/main.

Verdict

Clean, provably behavior-preserving extraction. No blockers, no should-fix. Two nits below, author's discretion.

Why I'm confident it's behavior-preserving

Each call site is a mechanical, semantics-identical substitution — motionSafe(p) ≡ reducedMotion.current ? { duration: 0 } : p:

site	before	after (expands to)
FilterToolbar slide	… ? {duration:0} : {duration:140}	motionSafe({duration:140}) → same
Toaster in	… ? {duration:0} : {y:-8,duration:300}	motionSafe({y:-8,duration:300}) → same
Toaster out	… ? {duration:0} : {duration:300}	motionSafe({duration:300}) → same

Reactivity is preserved: motionSafe reads reducedMotion.current inside the $derived, so the derived still tracks the MediaQuery dependency and re-runs when the preference flips — identical to reading it inline before.

Test genuinely fails without the change (Rule 10) ✅

Mutation-checked the "collapses to an instant cut" test against the exact bug the PR claims to prevent — a partial gate ({ ...params, duration: 0 }, i.e. zero duration but keep the y offset):

× collapses to an instant cut under reduced motion — no leftover offsets
  AssertionError: expected { y: -8, duration: +0 } to deeply equal { duration: +0 }

The test fails exactly when the a11y contract is weakened. Good test.

Gates — clean worktree off origin/main, fresh .svelte-kit

• lint (prettier + eslint) ✅
• check (svelte-check) — 0 errors / 0 warnings / 616 files ✅
• test (vitest) — motion.test.ts both branches ✅
• knip (check:unused) — clean; note reducedMotion import stays in Toaster.svelte and is still used at line 65 (item.duration > 0 && !reducedMotion.current), so it is not a dead import ✅
• build (vite + svelte-package) ✅ exit 0

⚠️ Flake note (not a PR defect): running npm run build twice back-to-back without cleaning .svelte-kit/dist in this shared worktree threw a one-off ERR_MODULE_NOT_FOUND: .../manifest-full.js. A clean build passes deterministically. Stale-output artifact of the shared checkout, not this change.

e2e

No browser-e2e harness is wired in this repo (only vitest unit + storybook; no Playwright script). Given the substitution is provably identical at every call site and the reduced-motion contract is covered by a mutation-verified unit test, I did not stand up an ad-hoc browser run — the static equivalence + unit coverage is stronger evidence here than a hand-driven emulation. Flagging honestly rather than claiming an e2e I didn't run.

Nits (discretion)

1. motion.ts sits in src/lib/actions/ but isn't a Svelte action (no use: directive) — it's a params helper. This matches the sibling reducedMotion.ts (also not an action), so it conforms to the existing convention; no change needed unless the folder later gets a stricter meaning.
2. Keeping it internal (not in the public index.ts) is the right call — exporting for consumers is a separate API/product decision. Agreed with the flag-don't-decide.

Disposition: approve-with-nits. Merge remains a human/board action per the never-merge rule.

[nsollazzo]

Review — R. Cutie (Positronick QA) · disposition: approve-with-nits (no blockers)

Reviewed the whole diff against the code it touches, in an isolated worktree at PR head 3cb73cb, off origin/main.

Behavior preservation — verified

motionSafe(p) = reducedMotion.current ? { duration: 0 } : p substitutes exactly into each prior inline gate:

• FilterToolbar slide: … ? {duration:0} : {duration:140} ≡ motionSafe({duration:140})
• Toaster in: … ? {duration:0} : {y:-8,duration:300} ≡ motionSafe({y:-8,duration:300})
• Toaster out: … ? {duration:0} : {duration:300} ≡ motionSafe({duration:300})

Behavior-preserving by construction — the browser can't reveal a divergence the algebra rules out.

Reactivity preserved

reducedMotion.current (a MediaQuery getter) is read synchronously inside motionSafe, which is called inside $derived(...). Svelte 5 tracks reactive reads during synchronous derivation, so the derived still re-subscribes and re-runs when the preference flips at runtime. No async boundary that would drop the dependency.

Scope is surgical

Exactly the 3 transition-params copies were extracted. The other reducedMotion.current gates (Banner, RainBackdrop, ScrollTopButton, and Toaster.svelte:65 {#if item.duration > 0 && !reducedMotion.current}) are a different pattern and correctly untouched. The reducedMotion import in Toaster.svelte is retained because line 65 still uses it directly — not a dead import (knip agrees).

Test genuinely fails without the change

motion.test.ts covers both branches. I mutation-tested it: swapping the helper to a partial gate ({ ...params, duration: 0 }) turns test 2 red ({duration:0, y:-8} ≠ {duration:0}) — it catches the exact "left a y offset" failure mode the doc-comment calls out. Meaningful, not tautological.

Gates (re-run in this worktree)

• lint (prettier + eslint) ✅
• check (svelte-check) ✅ 0 errors / 0 warnings / 616 files
• test ✅ 72 files / 302 passed — includes the .svelte.test.ts suites for Toaster & FilterToolbar and the *.reduced.svelte.test.ts suites, which run in headless chromium (Playwright client project). That is the real-browser pass.
• build (vite build + svelte-package) ✅
• knip (check:unused) ✅ clean

Nit (author discretion, non-blocking)

No browser test directly asserts the collapsed transition object for Toaster/FilterToolbar under prefers-reduced-motion. This gap is pre-existing (the old inline gates were never browser-asserted either) and net coverage improved here via motion.test.ts. If you want defense-in-depth, a Toaster.reduced.svelte.test.ts asserting an instant cut would close it — optional follow-up, not required for this PR.

No secrets, no security surface, no error-handling paths (pure total function over its typed input). Clean refactor.

Do not merge on my say-so — merge is a human/board action. This waits for a human.