fix(waapi): commit multi-word final keyframes to inline style - #75
Open
JonathonRP wants to merge 1 commit into
Open
fix(waapi): commit multi-word final keyframes to inline style#75JonathonRP wants to merge 1 commit into
JonathonRP wants to merge 1 commit into
Conversation
setProperty lowercases but does not kebab-case the property name, so committing the final keyframe of clipPath, backgroundColor and every other multi-word acceleratedValue was silently dropped and the element snapped back on cancel. Assign through the camelCase indexer like upstream, keeping CSS custom properties on setProperty. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 138d5e2 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 |
Contributor
📝 WalkthroughWalkthroughThe WAAPI style utility now directly assigns standard CSS properties and retains ChangesWAAPI CSS assignment
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
commit: |
![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: 1
🤖 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/motion-start/src/animation/animators/waapi/utils/style.ts`:
- Around line 16-20: Normalize custom-property names in setCSSVar so names
already beginning with “--” are not prefixed again. Update the completion path
used by NativeAnimation while preserving normal CSS property handling and
ensuring values are written to the intended custom property when WAAPI is
unavailable.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b745aa45-6505-40bd-b9ad-8a225f4d6039
📒 Files selected for processing (3)
.changeset/wild-poems-commit.mdpackages/motion-start/src/animation/animators/waapi/__tests__/animate-style.spec.tspackages/motion-start/src/animation/animators/waapi/utils/style.ts
Comment on lines
+16
to
+20
| if (name.startsWith('--')) { | ||
| element.style.setProperty(name, String(value)); | ||
| } else { | ||
| element.style[name as MotionStyleKey] = value as never; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'packages/motion-start/src/animation/animators/waapi/(style|NativeAnimation)\.ts$' || true
echo
echo "style.ts outline and contents:"
if [ -f packages/motion-start/src/animation/animators/waapi/utils/style.ts ]; then
ast-grep outline packages/motion-start/src/animation/animators/waapi/utils/style.ts || true
cat -n packages/motion-start/src/animation/animators/waapi/utils/style.ts
fi
echo
echo "NativeAnimation.ts relevant sections:"
if [ -f packages/motion-start/src/animation/animators/waapi/NativeAnimation.ts ]; then
wc -l packages/motion-start/src/animation/animators/waapi/NativeAnimation.ts
ast-grep outline packages/motion-start/src/animation/animators/waapi/NativeAnimation.ts || true
sed -n '1,190p' packages/motion-start/src/animation/animators/waapi/NativeAnimation.ts | cat -n
fi
echo
echo "search setCSSVar/setStyle usages:"
rg -n "setCSSVar|setStyle|valueName|style\.setProperty|custom property|--foo|----foo" packages/motion-start/src/animation/animators/waapi packages/motion-start/src -S || true
echo
echo "test files mentioning custom property setCSSVar:"
rg -n "setCSSVar|--\w+|custom" packages/motion-start -g '*test*' -g '*spec*' -g '*.test.ts' -g '*.test.tsx' -S || trueRepository: JonathonRP/motion-start
Length of output: 27835
Normalize CSS custom-property names before setCSSVar.
NativeAnimation passes the full custom-property name such as --css-var with valueName.startsWith('--'). setCSSVar prefixes another --, so completion writes ----css-var when WAAPI is unavailable. Accept names as --css-var and avoid double-prefixing in setCSSVar, or strip one prefix before the call.
🤖 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/motion-start/src/animation/animators/waapi/utils/style.ts` around
lines 16 - 20, Normalize custom-property names in setCSSVar so names already
beginning with “--” are not prefixed again. Update the completion path used by
NativeAnimation while preserving normal CSS property handling and ensuring
values are written to the intended custom property when WAAPI is unavailable.
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
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.
The bug
setStyleinpackages/motion-start/src/animation/animators/waapi/utils/style.tscommitted the final keyframe withelement.style.setProperty(name, String(value)).CSSOM's
setPropertylowercases the property name it is given but does not convert camelCase to kebab-case, sosetProperty("clipPath", ...)becomes"clippath", which is not a real property, and the declaration is silently discarded.NativeAnimation.onFinishwrites the final keyframe withsetValueand then immediately cancels the WAAPI animation. When the write is a no-op, cancelling drops the element straight back to its pre-animation value — exactly the flash the commit exists to prevent.clipPath,backgroundColor,borderRadiusand every other multi-word member ofacceleratedValuestake the WAAPI path, so all of them were affected.It went unnoticed because the only coverage was
opacity, a single-word property whose name survivessetPropertyunchanged.The fix
Assign through the camelCase indexer, matching upstream framer-motion v11.11.11:
element.style[name as MotionStyleKey] = value as never. CSS custom properties (--*) still go throughsetProperty, since the indexer does not work for those.Tests
Added two regression tests alongside the existing
opacitycases inwaapi/__tests__/animate-style.spec.ts, coveringclipPathandbackgroundColorand asserting the final keyframe lands in inline style after the animation finishes and is cancelled. Both fail against the old implementation (expected '' to be 'inset(10%)') and pass with the fix; the existingopacitycoverage stays green.vitest --runonpackages/motion-start: 620 passed. Two unrelated specs (reorder-production-ssr,package-imports) hit the 30s timeout under parallel load on this machine and pass in isolation.A patch changeset is included.
Summary by CodeRabbit
Bug Fixes
clipPath,backgroundColor, andborderRadius.Tests