Add AnimatePresence + MotionStyle/CSSValue interp #70
Merged
Conversation
When a motion handle has been idle, the web loop can sleep for the idle poll interval. If a short exit tween starts during that sleep, the first update inherited the whole idle delta and could complete immediately, making initially visible elements disappear without a visible exit transition. Reset the frame clock when transitioning from idle to running and keep web running frames on an 8ms cadence. Presence-driven helpers now wait until the exit animation has actually been observed running before calling safe_to_remove, while preserving immediate removal for zero-duration exits.
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds ChangesAnimatePresence + MotionStyle Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant AnimatePresence
participant PresenceContext
participant ChildComponent
participant use_presence_style
User->>AnimatePresence: render with keyed children
AnimatePresence->>PresenceContext: register keyed child (entering)
PresenceContext-->>ChildComponent: provide PresenceHandle + is_present=true
ChildComponent->>use_presence_style: use_presence_style(presence_style!{...})
use_presence_style-->>ChildComponent: animate to "animate" state
User->>AnimatePresence: remove keyed child from tree
AnimatePresence->>PresenceContext: mark child as exiting (retain in DOM)
PresenceContext-->>ChildComponent: is_present=false
ChildComponent->>use_presence_style: animate to "exit" state
use_presence_style->>PresenceContext: safe_to_remove() on completion
PresenceContext->>AnimatePresence: unmount retained child
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/package.json (1)
10-11:⚠️ Potential issue | 🟠 MajorAlign Tailwind dependency versions with the committed generated CSS.
docs/package.jsonpins@tailwindcss/cliandtailwindcssto^4.1.11, butdocs/assets/main.csswas generated bytailwindcss v4.3.1(visible in the file header). This mismatch breaks rebuild reproducibility—regenerating the CSS will produce different output.Proposed fix
"dependencies": { - "`@tailwindcss/cli`": "^4.1.11", - "tailwindcss": "^4.1.11" + "`@tailwindcss/cli`": "^4.3.1", + "tailwindcss": "^4.3.1" }🤖 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 `@docs/package.json` around lines 10 - 11, The tailwindcss dependencies in docs/package.json are pinned to version ^4.1.11, but the generated CSS file docs/assets/main.css was created using tailwindcss v4.3.1 as indicated in its header comment. Update both the `@tailwindcss/cli` and tailwindcss version entries in docs/package.json from ^4.1.11 to ^4.3.1 to align with the version that generated the committed CSS file, ensuring rebuild reproducibility and preventing future CSS regeneration from producing different output.
🧹 Nitpick comments (1)
src/animations/style.rs (1)
210-213: ⚡ Quick winNormalize keys in
property()for consistency withadd_css_property().Right now
property()can bypass normalization and create parallel keys for the same CSS property.Suggested refactor
pub fn property(mut self, property: impl Into<String>, value: CssValue) -> Self { - self.properties.insert(property.into(), value); + let property = normalize_style_property(&property.into()); + self.properties.insert(property, value); self }🤖 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 `@src/animations/style.rs` around lines 210 - 213, The property() method currently inserts CSS property keys without normalization, which can create duplicate entries for the same property compared to add_css_property(). Apply the same key normalization logic that add_css_property() uses when inserting the property into self.properties, ensuring that keys are normalized before being used as map keys to maintain consistency across both methods.
🤖 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 `@docs/assets/main.css`:
- Around line 551-563: The stylelint configuration is currently checking
docs/assets/main.css which is a generated Tailwind output file, causing
declaration-empty-line-before violations to be reported on auto-generated
utility blocks like .-translate-x-4, .-translate-x-\[100px\], and
.translate-x-0. Since this file is regenerated on each build, exclude
docs/assets/main.css from stylelint checks by modifying the stylelint
configuration (typically .stylelintrc or stylelint config in package.json) to
either ignore this generated file path or to only lint source Tailwind input
files rather than the compiled output.
In `@docs/src/pages/presence_guide.rs`:
- Around line 756-758: The detail text for the `initial: false` property at line
756 incorrectly states it applies to all "newly mounted children," but the
actual implementation only suppresses the initial animation on first
render—children inserted later still run their enter animation. Update the
detail string to accurately reflect that initial: false only affects the initial
render behavior and clarifies that subsequently mounted children will still
animate on entry.
In `@src/animations/css.rs`:
- Around line 559-579: The parse_rgb_color function incorrectly handles
percentage values in RGB and alpha channels. When parsing color values, the code
strips the percent sign with trim_end_matches but does not scale the values. For
RGB percentages, values should be scaled from 0-100% to 0-255 range, and for
alpha percentages, values should be scaled from 0-100% to 0-1 range. Modify the
parsing logic to detect when a part contains a percent sign, then apply the
appropriate scaling factor based on whether it's an RGB channel (first 3 parts,
multiply by 2.55) or an alpha channel (4th part, divide by 100).
- Around line 219-236: The CssComplexValue::parse method uses byte indexing with
the `index` variable to iterate through the value string, but in the else branch
it advances by only 1 byte with index += 1 after pushing a character. For
non-ASCII characters which are multiple bytes in UTF-8, this causes the next
value[index..] slice to potentially index in the middle of a multi-byte
character, causing a panic. Replace the byte-indexing approach with
character-aware iteration using char_indices() or a similar method that returns
both the character and its correct byte position, so that the index is always
positioned at valid UTF-8 boundaries.
In `@src/animations/style.rs`:
- Around line 480-492: The interpolation logic only processes properties from
the target style, leaving properties that exist only in the start style
(self.properties) in an unstable state. After the existing loop that handles
target properties, add additional logic to iterate over properties in
self.properties that are not present in target.properties, and ensure these
self-only properties interpolate correctly by inserting them back into
style.properties with the appropriate interpolated value (they should
interpolate to themselves to remain stable throughout the animation).
In `@src/lib.rs`:
- Around line 136-143: The prelude module contains a duplicate re-export of
MotionStyle from crate::animations::style. The MotionStyle symbol is currently
re-exported twice, which causes a compilation error E0252. Remove the second pub
use statement that re-exports MotionStyle to eliminate the duplicate declaration
in the module namespace.
---
Outside diff comments:
In `@docs/package.json`:
- Around line 10-11: The tailwindcss dependencies in docs/package.json are
pinned to version ^4.1.11, but the generated CSS file docs/assets/main.css was
created using tailwindcss v4.3.1 as indicated in its header comment. Update both
the `@tailwindcss/cli` and tailwindcss version entries in docs/package.json from
^4.1.11 to ^4.3.1 to align with the version that generated the committed CSS
file, ensuring rebuild reproducibility and preventing future CSS regeneration
from producing different output.
---
Nitpick comments:
In `@src/animations/style.rs`:
- Around line 210-213: The property() method currently inserts CSS property keys
without normalization, which can create duplicate entries for the same property
compared to add_css_property(). Apply the same key normalization logic that
add_css_property() uses when inserting the property into self.properties,
ensuring that keys are normalized before being used as map keys to maintain
consistency across both methods.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88d44ac7-e28c-4d7a-ae99-8fb979adf2e9
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockdocs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
Cargo.tomldocs/assets/main.cssdocs/package.jsondocs/src/pages/docs/index.rsdocs/src/pages/mod.rsdocs/src/pages/motion_style_guide.rsdocs/src/pages/presence_guide.rsdocs/src/utils/router.rsdocs/tests/presence_layout_height.mjsspecs/animate-presence-spec.mdsrc/animations/core.rssrc/animations/css.rssrc/animations/mod.rssrc/animations/style.rssrc/lib.rssrc/manager.rssrc/motion.rssrc/pool.rssrc/presence.rssrc/presence_macros.rssrc/sequence.rssrc/style_macros.rs
Comment on lines
+551
to
+563
| translate: var(--tw-translate-x) var(--tw-translate-y); | ||
| } | ||
| .-translate-x-4 { | ||
| --tw-translate-x: calc(var(--spacing) * -4); | ||
| translate: var(--tw-translate-x) var(--tw-translate-y); | ||
| } | ||
| .-translate-x-\[100px\] { | ||
| --tw-translate-x: calc(100px * -1); | ||
| translate: var(--tw-translate-x) var(--tw-translate-y); | ||
| } | ||
| .translate-x-0 { | ||
| --tw-translate-x: 0; | ||
| translate: var(--tw-translate-x) var(--tw-translate-y); |
Contributor
There was a problem hiding this comment.
Stylelint is failing on generated Tailwind output.
Static analysis reports declaration-empty-line-before violations on these generated utility blocks. Since this file is generated, manual formatting edits will be overwritten on the next build. Prefer excluding docs/assets/main.css from stylelint (or lint only source Tailwind inputs) to keep CI stable.
Also applies to: 677-677, 719-719, 733-733, 1320-1320, 1467-1471
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 551-551: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 555-555: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 559-559: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 563-563: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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 `@docs/assets/main.css` around lines 551 - 563, The stylelint configuration is
currently checking docs/assets/main.css which is a generated Tailwind output
file, causing declaration-empty-line-before violations to be reported on
auto-generated utility blocks like .-translate-x-4, .-translate-x-\[100px\], and
.translate-x-0. Since this file is regenerated on each build, exclude
docs/assets/main.css from stylelint checks by modifying the stylelint
configuration (typically .stylelintrc or stylelint config in package.json) to
either ignore this generated file path or to only lint source Tailwind input
files rather than the compiled output.
Source: Linters/SAST tools
wheregmis
reviewed
Jun 19, 2026
Owner
There was a problem hiding this comment.
Optional Comment: We can definately omit this specs file, if the goal of this file is achieved through the implementation
wheregmis
approved these changes
Jun 19, 2026
Owner
|
@sarendipitee I dont mind bringing this changes as it is, but if you think some critical nits reported by coderabit is something that we can address, i would love them to be addressed or closed based on your use case. Feel free to mention me on that. PS: Please fix the failed CIs |
Contributor
Author
|
nits have been de-picked :) CodeRabbit seems to be happy now |
Owner
Waiting for CI to go green, Will merge afterwards, Thank you for your contribution. Really appreciate it |
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
2 participants
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.
No description provided.