-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(spinner): fix variant spinner
animation
#5578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@yurkimus is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe "fade-out" animation in the animation definitions was updated to use a dynamic delay based on the CSS variable Changes
Sequence Diagram(s)sequenceDiagram
participant CSS
participant DOM_Element
CSS->>DOM_Element: Apply "fade-out" animation with delay calc(var(--bar-index) * 100ms)
Note right of DOM_Element: Animation starts staggered based on --bar-index value
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/theme/src/animations/index.ts (1)
9-9
: Fix looks correct; consider trimming redundant tokens for consistency.The updated shorthand now correctly injects the dynamic delay, resolving #5577.
However, unlike the other animation strings in this object (e.g."spinner-spin 0.8s ease infinite"
), this one explicitly lists the defaultdirection
,fill-mode
, andplay-state
(normal none running
). They are superfluous because those values are the browser defaults; keeping them inflates bundle size and introduces style‐guide inconsistency.-"fade-out": "fade-out 1.2s linear calc(var(--bar-index) * 100ms) infinite normal none running", +"fade-out": "fade-out 1.2s linear calc(var(--bar-index) * 100ms) infinite",Optional, but worth aligning with the rest of the file.
Spinner
variant spinner
animationspinner
animation
Closes
--bar-index
variable within the delay offade-out
animation in componentSpinner
of variantspinner
#5577📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit