feat(theme): add global animation configuration #3843
Conversation
…LESS-based) (#3632) * feat(theme): [transition] add global animation configuration scheme (LESS-based) * fix(tag): fix the index.less * feat(theme): [motion] add global animation variables * feat(theme): [motion] update the documentation and fix some bug * feat(theme): [motion] rename the documentation and remove unnecessary comments * feat(theme): [motion] reset the css variables * feat(theme): [motion] optimized animation effect configuration * feat(theme): [motion] rename configuration * feat(theme): [motion] fix problem in test:e2e
WalkthroughThis PR introduces a centralized, configurable motion system for TinyVue by creating a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/theme/src/tabs/index.less (1)
1376-1518: Remove dead keyframe definitions (old fallback code).Lines 1376–1518 contain the original
slideInRight-enter,slideInRight-leave,slideInLeft-enter, andslideInLeft-leavekeyframes. Since the animation references at lines 1355, 1362, 1366, and 1373 now point to the centralizedtab-slide-*-in/outdefinitions from the motion module, these old keyframes are no longer used and should be removed to eliminate dead code and reduce maintenance burden.Apply this diff to remove the unused keyframes:
.slideInLeft-leave { position: absolute; left: 0; right: 0; animation: tab-slide-left-out var(--tv-motion-tab-speed); } - - @-webkit-keyframes slideInRight-enter { - 0% { - opacity: 0; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(100%); - transform: translateX(100%); - } - - to { - opacity: 1; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - } - } - - @keyframes slideInRight-enter { - 0% { - opacity: 0; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(100%); - transform: translateX(100%); - } - - to { - opacity: 1; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - } - } - - @-webkit-keyframes slideInRight-leave { - 0% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - opacity: 1; - } - - 100% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(100%); - transform: translateX(100%); - opacity: 0; - } - } - - @keyframes slideInRight-leave { - 0% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - opacity: 1; - } - - 100% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(100%); - transform: translateX(100%); - opacity: 0; - } - } - - @-webkit-keyframes slideInLeft-enter { - 0% { - opacity: 0; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(-100%); - transform: translateX(-100%); - } - - to { - opacity: 1; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - } - } - - @keyframes slideInLeft-enter { - 0% { - opacity: 0; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(-100%); - transform: translateX(-100%); - } - - to { - opacity: 1; - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - } - } - - @-webkit-keyframes slideInLeft-leave { - 0% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - opacity: 1; - } - - 100% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(-100%); - transform: translateX(-100%); - opacity: 0; - } - } - - @keyframes slideInLeft-leave { - 0% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(0); - transform: translateX(0); - opacity: 1; - } - - 100% { - -webkit-transform-origin: 0 0; - transform-origin: 0 0; - -webkit-transform: translateX(-100%); - transform: translateX(-100%); - opacity: 0; - } - } }
🧹 Nitpick comments (3)
examples/sites/demos/pc/webdoc/motion-configuration-en.md (1)
1-183: Excellent documentation for the new motion system.This comprehensive documentation clearly explains the centralized motion configuration approach, including global variables, directory structure, examples, and maintenance guidelines. The examples for fade, slide, and ants animations are particularly helpful.
The directory structure fenced code block on lines 48-63 is missing a language specifier. While it renders fine, adding a language identifier would satisfy the linter and improve consistency:
-``` +```text motion/ ├─ fade.less // Fade in/out ...</blockquote></details> <details> <summary>examples/sites/demos/pc/webdoc/motion-configuration.md (1)</summary><blockquote> `1-183`: **优秀的动效系统文档。** This comprehensive Chinese documentation effectively explains the centralized motion configuration system with clear examples and guidelines. The directory structure fenced code block on lines 48-63 is missing a language specifier. Adding it would satisfy the linter: ```diff -``` +```text motion/ ├─ fade.less // 淡入淡出 ...</blockquote></details> <details> <summary>packages/theme/src/motion/progress.less (1)</summary><blockquote> `19-26`: **Optional: Consider simplifying webkit prefix maintenance.** Modern browsers (including Chrome 43+, Safari 9+, Edge 12+) support standard `@keyframes` without the `-webkit-` prefix. If the project targets recent browsers, the webkit-prefixed version could be removed to reduce duplication. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 94f7b919a21064c8fac577c157ff0e8d362a73e4 and 8828eebc112e49706414e93aca79c8617b9788ca. </details> <details> <summary>📒 Files selected for processing (41)</summary> * `examples/sites/demos/pc/app/scroll-text/scroll-direction.spec.ts` (1 hunks) * `examples/sites/demos/pc/menus.js` (1 hunks) * `examples/sites/demos/pc/webdoc/motion-configuration-en.md` (1 hunks) * `examples/sites/demos/pc/webdoc/motion-configuration.md` (1 hunks) * `packages/theme/src/base/index.less` (1 hunks) * `packages/theme/src/base/motion-theme.less` (1 hunks) * `packages/theme/src/base/reset.less` (1 hunks) * `packages/theme/src/base/transition.less` (1 hunks) * `packages/theme/src/base/vars.less` (1 hunks) * `packages/theme/src/chart-core/index.less` (2 hunks) * `packages/theme/src/dialog-box/index.less` (2 hunks) * `packages/theme/src/drawer/index.less` (2 hunks) * `packages/theme/src/fall-menu/index.less` (1 hunks) * `packages/theme/src/grid/body.less` (3 hunks) * `packages/theme/src/grid/excel.less` (1 hunks) * `packages/theme/src/grid/icon.less` (2 hunks) * `packages/theme/src/grid/loading.less` (2 hunks) * `packages/theme/src/image/index.less` (2 hunks) * `packages/theme/src/motion/ants.less` (1 hunks) * `packages/theme/src/motion/arrow.less` (1 hunks) * `packages/theme/src/motion/bounce.less` (1 hunks) * `packages/theme/src/motion/fade.less` (1 hunks) * `packages/theme/src/motion/index.less` (1 hunks) * `packages/theme/src/motion/loading.less` (1 hunks) * `packages/theme/src/motion/progress.less` (1 hunks) * `packages/theme/src/motion/rotate.less` (1 hunks) * `packages/theme/src/motion/scroll.less` (1 hunks) * `packages/theme/src/motion/shine.less` (1 hunks) * `packages/theme/src/motion/slide.less` (1 hunks) * `packages/theme/src/motion/stroke.less` (1 hunks) * `packages/theme/src/motion/tab.less` (1 hunks) * `packages/theme/src/motion/zoom.less` (1 hunks) * `packages/theme/src/popup/index.less` (1 hunks) * `packages/theme/src/progress/index.less` (2 hunks) * `packages/theme/src/scroll-text/index.less` (3 hunks) * `packages/theme/src/skeleton-item/index.less` (2 hunks) * `packages/theme/src/tabs/index.less` (2 hunks) * `packages/theme/src/transition/carousel-arrow.less` (3 hunks) * `packages/theme/src/transition/icon.less` (1 hunks) * `packages/theme/src/transition/timepicker.less` (1 hunks) * `packages/theme/src/tree/index.less` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2024-11-04T09:35:13.159Z</summary>Learnt from: zzcr
Repo: opentiny/tiny-vue PR: 2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在packages/theme/src/time-range/vars.less文件中，应使用var(--tv-TimeRange-header-height)作为--tv-TimeRange-header-line-height的值，以保持一致性。**Applied to files:** - `packages/theme/src/base/index.less` - `packages/theme/src/base/reset.less` - `packages/theme/src/transition/timepicker.less` - `packages/theme/src/motion/scroll.less` - `packages/theme/src/motion/tab.less` - `packages/theme/src/motion/progress.less` - `packages/theme/src/grid/icon.less` - `packages/theme/src/base/motion-theme.less` - `packages/theme/src/motion/shine.less` - `packages/theme/src/motion/fade.less` - `packages/theme/src/transition/icon.less` - `packages/theme/src/base/vars.less` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>examples/sites/demos/pc/webdoc/motion-configuration-en.md</summary> 48-48: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>examples/sites/demos/pc/webdoc/motion-configuration.md</summary> 48-48: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: PR E2E Test (pnpm test:e2e3) </details> <details> <summary>🔇 Additional comments (41)</summary><blockquote> <details> <summary>packages/theme/src/base/index.less (1)</summary><blockquote> `17-17`: **Commented import has no current effect.** This commented import reference to `motion-theme.less` appears to be placeholder for future theme-specific motion customizations. Since it's commented out, it has no runtime impact. </blockquote></details> <details> <summary>examples/sites/demos/pc/app/scroll-text/scroll-direction.spec.ts (1)</summary><blockquote> `6-9`: **LGTM! Test expectations correctly updated.** The animation name changes from `moveLeft/moveRight/moveUp/moveDown` to `scroll-left/scroll-right/scroll-up/scroll-down` align with the centralized motion system's naming conventions and maintain consistency across the codebase. </blockquote></details> <details> <summary>packages/theme/src/grid/icon.less (2)</summary><blockquote> `16-16`: **LGTM! Import enables centralized motion tokens.** The import of the motion module is appropriate for accessing centralized animation definitions. --- `289-292`: **LGTM! CSS variable enables configurable rotation speed.** The migration from hardcoded `rollCircle 1s infinite linear` to `rotate-spin var(--tv-motion-rotate-speed) infinite linear` with a configurable CSS variable is a good improvement. The `rotate-spin` keyframes are properly defined in `packages/theme/src/motion/rotate.less` and allow runtime customization of both rotation speed and degrees. </blockquote></details> <details> <summary>examples/sites/demos/pc/menus.js (1)</summary><blockquote> `56-63`: **LGTM! Menu entry properly configured.** The new documentation menu item for global motion configuration is appropriately placed between theme-related entries and correctly configured with bilingual titles. The `showScene` restriction to the 'default' theme is appropriate since motion configuration is theme-specific. </blockquote></details> <details> <summary>packages/theme/src/fall-menu/index.less (1)</summary><blockquote> `188-188`: **The keyframes "leftArrow" and "rightArrow" do not exist in the motion module.** Investigation shows: - The motion module (packages/theme/src/motion/arrow.less) contains arrow animations with different naming: `arrow-left-pulse`, `arrow-right-pulse`, `arrow-left-carousel`, `arrow-right-carousel`, `arrow-left-in`, `arrow-right-in`, `arrow-left-out`, `arrow-right-out` - The fall-menu component does not reference or use any arrow animations currently - No animation declarations or motion module imports are present in the fall-menu file The review comment assumes specific keyframes were removed, but no such keyframes with those names exist in the centralized motion module. The concern cannot be verified as stated. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>packages/theme/src/base/transition.less (1)</summary><blockquote> `5-5`: **Remove verification request—keyframes don't exist in the codebase.** The verification request cannot be completed as stated. An exhaustive search of the theme module found no evidence that `modal-fade-in` or `modal-fade-out` keyframes ever existed in `transition.less` or were migrated to the motion module. These keyframes are not present anywhere in the codebase, and no components reference them. The import of the motion module in `transition.less` is appropriate for the existing transition utilities. </blockquote></details> <details> <summary>packages/theme/src/base/motion-theme.less (1)</summary><blockquote> `1-622`: **LGTM: Comprehensive centralized design token system.** This new file establishes a well-organized foundation for the motion system refactor by defining CSS custom properties for colors, typography, spacing, sizes, shadows, and animations. The motion section (lines 526–621) provides configurable parameters for all animation types with clear documentation and sensible defaults, enabling the motion modules to reference these tokens consistently. </blockquote></details> <details> <summary>packages/theme/src/progress/index.less (1)</summary><blockquote> `15-15`: **LGTM: Clean motion import addition.** Import order is correct and positions the motion module before usage context. </blockquote></details> <details> <summary>packages/theme/src/motion/stroke.less (1)</summary><blockquote> `1-46`: **LGTM: Well-structured stroke animation module.** Proper documentation, clear fallback values for all CSS variables, and appropriate two-direction animation support. </blockquote></details> <details> <summary>packages/theme/src/grid/excel.less (1)</summary><blockquote> `14-14`: **LGTM: Motion import addition.** Correctly positioned after custom imports. </blockquote></details> <details> <summary>packages/theme/src/motion/zoom.less (1)</summary><blockquote> `1-68`: **LGTM: Comprehensive zoom animation module.** Provides both fade and non-fade variants for zoom in/out animations. Note: The fallback value for `--tv-motion-scale-mid` is 0.5 (lines 20, 34, 48, 62), which differs from the theme declaration of 1.2 in motion-theme.less (line 617). Verify this is intentional—if the theme value should take precedence, consider removing the fallback here and relying on the theme definition. </blockquote></details> <details> <summary>packages/theme/src/base/reset.less (1)</summary><blockquote> `15-20`: **LGTM: Correct animation syntax pattern.** Unlike popup/index.less and transition/timepicker.less, this file correctly orders the animation shorthand property with the animation name first (line 20). This is the pattern that should be applied across all refactored animation declarations. </blockquote></details> <details> <summary>packages/theme/src/transition/timepicker.less (1)</summary><blockquote> `17-21`: **CSS animation property syntax error: incorrect parameter order.** Same critical issue as popup/index.less. The animation name must come before the duration variable in the animation shorthand. Apply this diff to fix the animation syntax: ```diff .@{css-prefix}transition-timepicker { - animation: fade-in-timepicker var(--tv-motion-fade-speed); + animation: fade-in-timepicker var(--tv-motion-fade-speed) linear; } .@{css-prefix}transition-timepicker-up { - animation: fade-up-timepicker var(--tv-motion-fade-speed); + animation: fade-up-timepicker var(--tv-motion-fade-speed) linear; }Note: Added
linearas the timing function since none was specified in the original code. If a different timing function is intended, adjust accordingly.Likely an incorrect or invalid review comment.
packages/theme/src/image/index.less (1)
13-15: Centralized motion import and animation usage look good.The transition to using centralized fade animations with CSS variables is correct. Animation names
fade-in-upandfade-out-upshould be defined inmotion/fade.less, and the timing variable--tv-motion-fade-speedshould be available via the motion module's CSS variable definitions.To confirm these animations are properly defined, please verify that:
packages/theme/src/motion/fade.lessexports@keyframes fade-in-upand@keyframes fade-out-up- The CSS variable
--tv-motion-fade-speedis defined in the theming system (likely inpackages/theme/src/base/vars.less)Also applies to: 83-89
packages/theme/src/grid/loading.less (1)
13-14: Grid loading animation properly centralized.The transition to variable-driven
grid-loadinganimation with correct syntax is good. The animation variable--tv-motion-grid-loading-speedshould be defined in the CSS variable system.Verify that
packages/theme/src/motion/loading.lessexports the@keyframes grid-loadingdefinition and that--tv-motion-grid-loading-speedis available as a CSS custom property in the theme's variable system.Also applies to: 60-60
packages/theme/src/chart-core/index.less (1)
13-15: Chart core animations properly migrated to centralized motion system.The loader animations now correctly reference centralized definitions (
rotate-spinandstroke-dash-loop) with appropriate timing variables. The CSS syntax is correct.Confirm that:
packages/theme/src/motion/rotate.lessdefines@keyframes rotate-spinpackages/theme/src/motion/stroke.lessdefines@keyframes stroke-dash-loop- CSS variables
--tv-motion-rotate-speedand--tv-motion-stroke-speedare defined in the theme's variable systemAlso applies to: 52-65
packages/theme/src/motion/rotate.less (1)
1-30: Rotation keyframes well-implemented with configurable variables.The module provides clear, variable-driven rotation animations (
rotate-spin,rotate-z,rotate-reverse) with sensible defaults. The header comments documenting sources and configuration variables are helpful for maintainability.packages/theme/src/tabs/index.less (1)
1348-1374: Tab animations properly migrated to centralized motion system.The transition to
tab-slide-*animation names with CSS variable timing is correct and follows the motion system pattern.packages/theme/src/motion/index.less (1)
1-16: Motion aggregator module is well-structured and ready to merge.Verification confirms all 13 motion submodule files are present and there are no circular dependencies. The import structure is clean and follows best practices for a modular animation system.
packages/theme/src/grid/body.less (2)
13-14: Good: Motion module import properly placed after theme variables.The import follows the correct sequence (custom, component vars, then motion definitions).
59-87: All animations and variables are properly defined. Code verified as correct.Verification confirms:
- All four animation keyframes (ants-x, ants-y, ants-x-rev, ants-y-rev) are defined in
packages/theme/src/motion/ants.less- The
--tv-motion-ants-speedvariable is defined inpackages/theme/src/base/vars.less(line 529) with value0.8sThe directional ants animation implementation is complete and correct.
packages/theme/src/transition/carousel-arrow.less (2)
13-14: Good: Motion import placed correctly before animation usage.
23-43: Carousel arrow animations verified—keyframes and speed variable properly defined.✓ Arrow carousel keyframes (
arrow-left-carousel,arrow-right-carousel) are defined inpackages/theme/src/motion/arrow.less
✓ Speed variable (--tv-motion-arrow-speed: 0.3s) is defined inpackages/theme/src/base/vars.lessThe enter/leave animation pairs correctly use
reversefor leave states, following CSS animation best practices.packages/theme/src/motion/progress.less (1)
1-26: CSS variables confirmed to be properly defined.The animation uses CSS variables
--tv-motion-progress-background-shiftand--tv-motion-progress-speed, which are correctly defined inpackages/theme/src/base/vars.lessat lines 559–560. The webkit-prefixed keyframes ensure compatibility with older browsers. No issues found.packages/theme/src/motion/shine.less (1)
1-18: CSS variables are properly defined; code approved.The animation correctly references
--tv-motion-shine-offset-xand--tv-motion-shine-offset-y, both of which are defined inpackages/theme/src/base/vars.less(lines 575-576) with appropriate defaults. The code is well-structured and documented.packages/theme/src/motion/ants.less (1)
1-39: CSS variable--tv-motion-ants-shiftis properly defined.Verification confirms the variable exists at
packages/theme/src/base/vars.less:528with the expected default value of8px, matching the fallback used in the animations.packages/theme/src/motion/slide.less (1)
1-133: All motion slide CSS variables are properly defined.Verification confirms that packages/theme/src/base/vars.less contains all required variables (lines 579-590): animation speed, mid-opacity, and directional offsets for all four slide directions. The implementation is complete and well-supported.
packages/theme/src/motion/loading.less (1)
1-26: Good: Loading animations properly separate motion definition from timing, and all documented CSS variables are defined.The skeleton-loading and grid-loading animations are well-designed with properly separated concerns. The timing variables (
--tv-motion-skeleton-speed,--tv-motion-skeleton-background-offset,--tv-motion-grid-loading-speed,--tv-motion-grid-loading-opacity) are correctly defined inpackages/theme/src/base/vars.less(lines 552-556) and applied asanimation-durationin component styles, which follows correct CSS animation patterns.packages/theme/src/tree/index.less (1)
13-15: Keyframe mismatch: motion module does not contain the expected keyframes.Verification found that:
- motion/rotate.less contains
@keyframes rotate-spin(notloading-rotate)- motion/loading.less contains
@keyframes skeleton-loadingand@keyframes grid-loading(not the removed keyframes)- tree/index.less currently has no animation references
- The motion/rotate.less comment indicates
rotate-spinis intended to replaceloading-rotate, but the naming differs from the assumption in the reviewThe import of
../motion/index.lessappears incomplete or misaligned with actual keyframe definitions in the motion module. Either the keyframe names need to be updated to match the motion module exports, or the motion module keyframes need to be added/corrected to provideloading-rotateandloading-dash.packages/theme/src/transition/icon.less (1)
14-14: Verify motion/index.less aggregation and CSS variable definitions.The refactoring looks sound—hardcoded durations replaced with CSS variables and local keyframes removed. Ensure that:
motion/index.lessimports and re-exports thebounce-inkeyframe frombounce.less--tv-motion-bounce-speedis defined inbase/vars.lesswith an appropriate defaultpackages/theme/src/motion/tab.less (1)
1-114: Comprehensive tab animation module with well-designed configurability.The module provides solid directional coverage with paired in/out animations. All keyframes properly use CSS variables with sensible defaults. Documentation is clear and variable naming follows consistent conventions (
--tv-motion-tab-{property}-{state}).One minor optimization: consider using
translate3d()instead oftranslateX()/translateY()for better GPU acceleration, though the current approach is functionally correct.Verify that
base/vars.lessdefines all six configurable variables:--tv-motion-tab-speed,--tv-motion-tab-opacity-start,--tv-motion-tab-opacity-end,--tv-motion-tab-offset-x-start,--tv-motion-tab-offset-x-start-left,--tv-motion-tab-offset-y-start,--tv-motion-tab-offset-y-start-down,--tv-motion-tab-offset-y-end.packages/theme/src/drawer/index.less (1)
3-3: Drawer animations properly refactored to use centralized motion system.The refactoring is clean—animations consistently reference the new motion keyframes with variable-driven durations. The timing function (
linear) is appropriately preserved for slide transitions.Verify that:
motion/index.lessimports and re-exportsslide-right-in,slide-right-out,slide-left-in,slide-left-out,slide-bottom-in,slide-bottom-out,slide-top-in,slide-top-outkeyframes--tv-motion-slide-speedand--tv-motion-fade-speedare defined inbase/vars.lessAlso applies to: 216-254
packages/theme/src/motion/bounce.less (1)
1-36: Clean and well-designed bounce animation module.The module provides a classic bounce effect with proper in/out symmetry. Variable naming is consistent, defaults are sensible (0 → 1.2 → 1 creates natural overshoot), and the documentation is clear.
Verify that
base/vars.lessdefines all motion bounce variables:--tv-motion-bounce-scale-min,--tv-motion-bounce-scale-mid,--tv-motion-bounce-scale-max, and--tv-motion-bounce-speed.packages/theme/src/motion/arrow.less (1)
1-110: Comprehensive arrow animation module with multiple effect types.The module provides solid coverage of arrow animations (pulse, carousel, in/out) with consistent directional handling. Naming follows established patterns (
arrow-{direction}-{type}), and all keyframes properly use CSS variables with sensible pixel-based defaults for positioning.The pulse animations (lines 14–36) cleverly use left/right position for oscillation, while carousel and in/out variants combine opacity fades with translateY positioning—appropriate for different use contexts.
Verify that
base/vars.lessdefines all six arrow-related variables:--tv-motion-arrow-offset,--tv-motion-arrow-left-init,--tv-motion-arrow-left-mid,--tv-motion-arrow-right-init,--tv-motion-arrow-right-mid, and--tv-motion-arrow-speed.packages/theme/src/motion/scroll.less (1)
12-96: Scroll animation module with both standard and adaptive variants—well designed.The module provides both basic scroll animations and adaptive variants with clever midpoint easing. All keyframes properly use CSS variables with sensible defaults (100%, -100% for full-page scrolls). The adaptive variants (interpolating through 0 at 50%) enable smoother visual transitions compared to linear scrolls.
Verify that
base/vars.lessdefines all five configurable variables:--tv-motion-scroll-speed,--tv-motion-scroll-offset-x,--tv-motion-scroll-offset-x-end,--tv-motion-scroll-offset-y, and--tv-motion-scroll-offset-y-end.packages/theme/src/dialog-box/index.less (2)
15-15: Dialog-box animations properly refactored with motion system integration.The animation refactoring is clean and consistent. Variable-driven durations replace hardcoded values, timing functions are appropriately maintained (
easefor fade,ease-infor slide,linearfor scale), and fill modes are correctly applied.
- Verify that
motion/index.lessimports and re-exports all referenced keyframes:fade-in,fade-out,fade-in-up,fade-out-up,slide-right-in,slide-right-out, andenlarge-in- Verify that
base/vars.lessdefines--tv-motion-fade-speed,--tv-motion-slide-speed, and--tv-motion-scale-speedAlso applies to: 212-241
232-236: Verify intentional animation duplication on nested selector.Lines 232–236 define the same animation (
fade-out-upwith--tv-motion-fade-speed) on both the parent.dialog-fade-leave-activeclass and the nested.@{dialog-box-prefix-cls}element. Confirm this dual-targeting is intentional or consolidate if redundant.packages/theme/src/motion/fade.less (1)
1-139: Comprehensive and well-designed fade animation module.The module provides excellent coverage: basic fades, four directional variants, and timepicker-specific animations. All keyframes properly use CSS variables with sensible defaults. The use of
translate3d()on directional variants (lines 32, 47, 54, 69, 76, 91, 98, 113) is a good performance optimization.The timepicker variants (lines 119–139) appropriately use fixed positioning with z-index management for drawer-like behavior, distinct from the configurable directional fades.
Naming is consistent (
fade-{direction}-{type}), documentation is clear, and the variable naming follows established conventions.Verify that
base/vars.lessdefines all three fade-related variables:--tv-motion-fade-speed,--tv-motion-fade-offset-x, and--tv-motion-fade-offset-y.packages/theme/src/base/vars.less (2)
526-622: Comprehensive motion design tokens added with consistent structure and clear documentation.The new variables establish a robust, centralized animation system across 13 categories. Naming, structure, and documentation are well-organized; values are semantically sound for CSS animations.
603-621: Reconcile discrepancy between Tab and Zoom scale variables.The AI summary lists Tab scale variables (
--tv-motion-tab-scale-start/mid/end), but the code shows scale variables only in the Zoom section (lines 614–621). Verify whether:
- Tab intentionally omits scale variables, or
- Tab should include scale variables alongside the opacity/offset variables currently present.
Additionally, confirm the
--tv-motion-scale-*naming (line 614 header referenceszoom.less) aligns with the zoom module's implementation intent.
| /* =========================== | ||
| 滚动文字动画 (scroll-text.less) | ||
| 来源: scroll-text |
There was a problem hiding this comment.
Fix comment inconsistency: module name mismatch.
Line 2 incorrectly references the filename as scroll-text.less when the actual file is scroll.less. Update the comment for clarity.
Apply this diff to fix the header comment:
/* ===========================
- 滚动文字动画 (scroll-text.less)
+ 滚动文字动画 (scroll.less)
来源: scroll-text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* =========================== | |
| 滚动文字动画 (scroll-text.less) | |
| 来源: scroll-text | |
| /* =========================== | |
| 滚动文字动画 (scroll.less) | |
| 来源: scroll-text |
🤖 Prompt for AI Agents
In packages/theme/src/motion/scroll.less around lines 1 to 3, the header comment
references the wrong filename (`scroll-text.less`) while the actual file is
`scroll.less`; update the second line of the comment to reference `scroll.less`
(or remove the filename) so the module name matches the file, preserving the
existing comment structure and any language/encoding.
| animation: var(--tv-motion-fade-speed) fade-in both ease-out; | ||
| } | ||
|
|
||
| &-leave-active { | ||
| animation: 0.3s mobile-fade-out both ease-in; | ||
| animation: var(--tv-motion-fade-speed) fade-out both ease-in; |
There was a problem hiding this comment.
CSS animation property syntax error: incorrect parameter order.
The CSS animation shorthand property requires the animation name as the first parameter, followed by duration. Lines 22 and 26 incorrectly place the duration variable before the animation name, which violates CSS syntax and will cause animations to fail.
Correct order: animation: <name> <duration> <timing-function> <fill-mode> ...
Apply this diff to fix the animation syntax:
&-enter-active {
- animation: var(--tv-motion-fade-speed) fade-in both ease-out;
+ animation: fade-in var(--tv-motion-fade-speed) ease-out both;
}
&-leave-active {
- animation: var(--tv-motion-fade-speed) fade-out both ease-in;
+ animation: fade-out var(--tv-motion-fade-speed) ease-in both;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| animation: var(--tv-motion-fade-speed) fade-in both ease-out; | |
| } | |
| &-leave-active { | |
| animation: 0.3s mobile-fade-out both ease-in; | |
| animation: var(--tv-motion-fade-speed) fade-out both ease-in; | |
| &-enter-active { | |
| animation: fade-in var(--tv-motion-fade-speed) ease-out both; | |
| } | |
| &-leave-active { | |
| animation: fade-out var(--tv-motion-fade-speed) ease-in both; | |
| } |
🤖 Prompt for AI Agents
In packages/theme/src/popup/index.less around lines 22 to 26, the animation
shorthand parameters are in the wrong order (duration placed before the
animation name); swap the animation name and the duration variable so the name
comes first (e.g., "fade-in var(--tv-motion-fade-speed) ease-out both") and do
the same for the leave-active rule using "fade-out" followed by the duration and
the existing timing-function and fill-mode.
| @import '../custom.less'; | ||
| @import './vars.less'; | ||
| @import '../motion/index.less'; |
There was a problem hiding this comment.
🧩 Analysis chain
Scroll animations properly migrated to centralized motion system.
The animation name changes from moveLeft/Right/Up/Down to scroll-left/right/up/down establish clearer naming conventions. The consistent use of animation-name properties is correct.
Verify:
- All
scroll-*animation keyframes are defined inpackages/theme/src/motion/scroll.less - The
singleUpanimation (line 71) is still defined locally or should be migrated to the motion system
Also applies to: 54-68, 100-108
🏁 Script executed:
#!/bin/bash
# Check if singleUp is defined in this file
rg -A 5 '@keyframes singleUp' packages/theme/src/scroll-text/index.less || echo "singleUp keyframes not found locally"
# Check if scroll animations are defined in motion
rg '@keyframes scroll-(left|right|up|down|left-adapt|right-adapt)' packages/theme/src/motion/Length of output: 562
🏁 Script executed:
#!/bin/bash
# Check singleUp usage in scroll-text/index.less around line 71
rg -n -B 2 -A 2 'singleUp' packages/theme/src/scroll-text/index.less
# Check if singleUp is defined anywhere else in the codebase
rg '@keyframes singleUp' packages/theme/src/Length of output: 88
🏁 Script executed:
#!/bin/bash
# Also search for any reference to singleUp being used
rg 'singleUp' packages/theme/src/Length of output: 136
---
🏁 Script executed:
```shell
#!/bin/bash
# Comprehensive search for singleUp definition in all .less files
fd -e less -x rg -l '@keyframes singleUp' {} \;
# Also check if it exists anywhere in the theme directory
find packages/theme -name '*.less' -exec rg -l 'singleUp' {} \;
Length of output: 41
Missing @keyframes singleUp definition breaks animation on line 71.
The .singleUp class references animation-name: singleUp, but this keyframe is not defined anywhere in the codebase. Unlike other animations migrated to the motion system, singleUp has no definition—neither locally nor in packages/theme/src/motion/. This will cause the animation to fail at runtime.
Fix required: Either restore the @keyframes singleUp definition or migrate it to the motion system following the pattern used for scroll-left, scroll-right, etc. (lines 54–68).
Location: packages/theme/src/scroll-text/index.less, lines 70–71
🤖 Prompt for AI Agents
In packages/theme/src/scroll-text/index.less around lines 13 and 70–71, the
.singleUp class references animation-name: singleUp but there is no @keyframes
singleUp defined (missing from this file and motion/), causing the animation to
fail; either add a matching @keyframes singleUp here or migrate/define it in
packages/theme/src/motion/ following the same pattern used for
scroll-left/scroll-right (create a motion file export or include the keyframes
with the same timing and transform steps used historically, then import that
motion file or update the .singleUp to reference the new motion keyframe name).
| @@ -1,5 +1,6 @@ | |||
| @import '../custom.less'; | |||
| @import './vars.less'; | |||
| @import '../motion/index.less'; | |||
There was a problem hiding this comment.
Critical: Animation property has incorrect syntax (duration before name).
Line 15 has the CSS animation properties in the wrong order. The duration variable appears before the animation name, which violates CSS animation shorthand syntax.
CSS animation property order is: animation: [name] [duration] [timing-function] [iteration-count] ...
This will cause the animation to fail or not apply at all.
Apply this fix to correct the animation syntax:
- animation: var(--tv-motion-skeleton-speed) skeleton-loading ease-in-out infinite;
+ animation: skeleton-loading var(--tv-motion-skeleton-speed) ease-in-out infinite;Also applies to: 15-15
🤖 Prompt for AI Agents
In packages/theme/src/skeleton-item/index.less around line 15, the CSS animation
shorthand has the duration variable placed before the animation name (invalid
order); update the shorthand so the animation-name appears first and the
duration second (e.g., change from " @duration @animation-name ..." to
"@animation-name @duration ..."), and ensure any timing-function, delay or
iteration-count follow in the correct order; apply this correction on line 15
(and any other identical occurrences).
zzcr
changed the title
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.