feat: wire up stagger for each's enter animations#35
Merged
Conversation
The stagger option on ListAnimationOptions was declared and exported but nothing in the runtime consumed it — each({ animate: { stagger: stagger(40) } }) produced no staggering. Fixed by threading index, totalItems, and a batch-start timestamp through IControlCtx.addSlot, into forkSlotEnter, which computes each slot's fire time relative to the shared reference and Effect.delays by only the residual after reconcile overhead. Compounding-delay bug (each item drifting further behind its expected position as reconcile takes time between slots) avoided by measuring from the shared start rather than from each slot's fork time.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying effex-api with
|
| Latest commit: |
855f30b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://00baf9ca.effex-api.pages.dev |
| Branch Preview URL: | https://feat-wire-up-stagger.effex-api.pages.dev |
Deploying effex with
|
| Latest commit: |
855f30b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f2a32a55.effex.pages.dev |
| Branch Preview URL: | https://feat-wire-up-stagger.effex.pages.dev |
…elpers from @effex/dom top level
The Animation namespace (group choreography), AnimationGroup type, and waitForAnimationEvent / waitForAnimationEnd / forceReflow helpers were reachable via @effex/dom/Animation but not from the top-level entry, so users had to know the internal module layout. Re-export them alongside the stagger helpers so 'import { Animation, ... } from "@effex/dom"' works.
Note: top-level sequence and parallel still refer to the deprecated Effect-combining helpers (kept for back-compat until the next major); the new choreography lives under Animation.sequence / Animation.parallel.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous stagger tests measured wall-clock timing of onBeforeEnter hooks. That's fine locally but flaky under CI load — Effect.delay(40ms) can fire at 200ms+ when setTimeout queues get starved, so 'item 1 fires < 70ms' fails intermittently.
Rewrote to spy on the stagger function itself: with three items, the function must be invoked with {index: 0, total: 3}, {1, 3}, {2, 3}. This proves the wiring (reconcile passes index/total through to forkSlotEnter which invokes the stagger function) without depending on any real-time behaviour. Second test verifies stagger isn't invoked when the option is omitted.
Also switched the Date.now() call in reconcile and forkSlotEnter to Clock.currentTimeMillis so callers can substitute a TestClock later if needed. Runtime behaviour is unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
staggerwas declared on `ListAnimationOptions` and exported via helpers (`stagger`, `staggerFromCenter`) but nothing in the runtime read it, so `each({ animate: { stagger: stagger(40) } })` produced no visible staggering. The API had been advertising the feature since before the current branch history without ever implementing it. This wires it up.Design note — why a shared reference timestamp
The naive approach — `Effect.delay(index * step)` per slot — compounds reconcile overhead:
Each item drifts further behind its expected position because reconcile itself takes time between forks (Signal.make, render, insertBefore all yield). Confirmed empirically in my first test iteration — item 1 fired at ~77ms with `stagger(40)`.
Fix: reconcile captures `batchStart = Date.now()` when a sync begins and threads it through `addSlot` → `forkSlotEnter`. Each slot's animation delays by `max(0, batchStart + stagger(index, total) - now())` — the residual after reconcile overhead. Slot N always fires at `batchStart + stagger(N, total)` regardless of how long reconcile takes to reach it.
Plumbing
Applies to `each` in all four paths (client, client-fallback-during-hydration, hydration-root, intro re-animation). `when` / `match` etc. don't take a stagger option — they render single elements.
Test plan
🤖 Generated with Claude Code