new: animation primitives for Solid 2.0#947
Open
davedbase wants to merge 9 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 15dcee0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/animation/README.md (1)
20-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the motion-path and sequence primitives here.
The package surface already exports
makeMotionPath/createMotionPathandmakeSequence, but this README stops atcreatePresenceAnimation. Please add table entries and matching sections so the shipped API is discoverable from the package landing page.Suggested diff
| [`makeAnimationGroup`](`#makeanimationgroup--createanimationgroup`) | Coordinate multiple animations as a unit | | [`createAnimationGroup`](`#makeanimationgroup--createanimationgroup`) | Reactive `makeAnimationGroup` | +| [`makeMotionPath`](`#makemotionpath--createmotionpath`) | Animate along a CSS `offset-path` | +| [`createMotionPath`](`#makemotionpath--createmotionpath`) | Reactive `makeMotionPath` | +| [`makeSequence`](`#makesequence`) | Chain animation factories sequentially | | [`createPresenceAnimation`](`#createpresenceanimation`) | Mount/unmount lifecycle with WAAPI enter/exit animations |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/animation/README.md` around lines 20 - 35, Add the missing documentation for the exported motion-path and sequence APIs in the animation README: update the primitives table to include makeMotionPath/createMotionPath and makeSequence, then add matching section anchors/details later in the document so these symbols are discoverable alongside createPresenceAnimation. Use the existing README entries for makeAnimate, createAnimationGroup, and createPresenceAnimation as the pattern for naming and linking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/animation/src/flip.ts`:
- Around line 27-34: The FLIP animation in flip() computes scale using
rect.width / next.width and rect.height / next.height without checking for
zero-sized target bounds. Add a guard before calculating sx and sy (or before
calling el.animate) to bail out when next.width or next.height is 0, and keep
the early return path in flip() so invalid transform values cannot be generated.
In `@packages/animation/src/motion-path.ts`:
- Line 28: The offset-path assignment in motion-path.ts is too aggressive
because it wraps every non-`path(...)` input into `path("...")`, which breaks
valid CSS values like `circle(...)` and `ray(...)`. Update the logic in the
motion-path setter so it preserves already-valid CSS `offset-path` values and
only normalizes raw SVG path strings when needed; use the existing `offsetPath`
handling in `motion-path.ts` as the place to make this distinction.
In `@packages/animation/src/presence-animation.ts`:
- Around line 26-30: The default exit path in reverseKeyframes and the
enter/exit handling in presence-animation.ts only reverse array-based keyframes,
so object-style PropertyIndexedKeyframes can still exit unreversed. Update
reverseKeyframes and the exit generation logic around the presence animation
flow so PropertyIndexedKeyframes are also transformed into a reversed form (or
otherwise handled equivalently) before being used for the default exit, while
preserving null and already-array behavior.
In `@README.md`:
- Line 142: The package catalog entry for the animation primitives is using the
wrong symbol name. Update the animation row in README to replace the nonexistent
createFlip reference with the actual exported primitive makeFlip, and ensure the
linked anchor points to the makeFlip documentation so the README matches the
package API.
---
Nitpick comments:
In `@packages/animation/README.md`:
- Around line 20-35: Add the missing documentation for the exported motion-path
and sequence APIs in the animation README: update the primitives table to
include makeMotionPath/createMotionPath and makeSequence, then add matching
section anchors/details later in the document so these symbols are discoverable
alongside createPresenceAnimation. Use the existing README entries for
makeAnimate, createAnimationGroup, and createPresenceAnimation as the pattern
for naming and linking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5672af77-f390-471f-8486-aaa245192458
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/animation-initial.mdREADME.mdpackages/animation/CHANGELOG.mdpackages/animation/LICENSEpackages/animation/README.mdpackages/animation/dev/index.tsxpackages/animation/package.jsonpackages/animation/src/animate.tspackages/animation/src/animation-group.tspackages/animation/src/flip.tspackages/animation/src/index.tspackages/animation/src/motion-path.tspackages/animation/src/presence-animation.tspackages/animation/src/scroll-animation.tspackages/animation/src/sequence.tspackages/animation/src/stagger.tspackages/animation/src/view-animation.tspackages/animation/stories/animation.stories.tsxpackages/animation/test/index.test.tspackages/animation/tsconfig.json
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.
This introduces a new package,
@solid-primitives/animation, providing reactive and imperative wrappers for the Web Animations API. All primitives follow the existingmake/createconvention:make*is imperative and returns immediately,create*is a reactive wrapper that re-runs on dependency change and cancels on owner disposal.Primitives:
makeAnimate/createAnimate— thin wrapper aroundelement.animate(), the reactive version re-runs whenevertarget,keyframes, oroptionschangemakeScrollAnimation/createScrollAnimation— scroll-driven animation viaScrollTimeline; no scroll listeners or RAF loops neededmakeViewAnimation/createViewAnimation— viewport-driven animation viaViewTimeline; replaces the IntersectionObserver + class-toggle pattern. IncludesrangeStart/rangeEndoptions (defaulting to"entry 0%"/"entry 100%") to scope animation to the entry phase onlymakeFlip— FLIP layout animation;snapshot()before the DOM change,flip()aftermakeStagger/createStagger— staggered WAAPI animation across a list of elements with a per-element delay offsetmakeAnimationGroup/createAnimationGroup— coordinates a list ofAnimationobjects as a single unit; all five control methods (play,pause,cancel,reverse,finish) forwarded simultaneously. The reactive variant re-derives the group viacreateMemowhenever the animation list changesmakeMotionPath/createMotionPath— animates an element along a CSSoffset-pathusingoffsetDistance; setsoffsetAnchor: centerso the element center tracks the pathmakeSequence— chains animation factories into a sequential playlist; factories are called lazily, each only when its predecessor finishesAlso includes a full Storybook stories file with interactive demos for each primitive, and a Vitest test suite covering the reactive wrappers, lifecycle cancellation, and option forwarding.
Summary by CodeRabbit
New Features
Documentation
Tests