feat: add animation effects to VbenModal component #6537
Conversation
Elm1992
requested review from
a team,
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
|
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DemoComponent
participant useVbenModal
participant Modal
participant DialogContent
User->>DemoComponent: Click "Open Slide Modal"
DemoComponent->>useVbenModal: open({ animationType: 'slide' })
useVbenModal->>Modal: show modal (animationType='slide')
Modal->>DialogContent: Pass animationType='slide'
DialogContent-->>User: Modal appears with slide animation
User->>DemoComponent: Click "Open Scale Modal"
DemoComponent->>useVbenModal: open({ animationType: 'scale' })
useVbenModal->>Modal: show modal (animationType='scale')
Modal->>DialogContent: Pass animationType='scale'
DialogContent-->>User: Modal appears with scale animation
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/src/components/common-ui/vben-modal.md(2 hunks)docs/src/demos/vben-modal/animation-type/index.vue(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal.ts(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal.vue(2 hunks)packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
docs/src/demos/vben-modal/animation-type/index.vue (3)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:31-31
Timestamp: 2024-12-04T04:41:30.161Z
Learning: 在 Vben Admin 项目的 `Page` 组件中，`extra` 内容仅通过 slots 提供，没有对应的 prop 属性。
packages/@core/ui-kit/popup-ui/src/modal/modal.vue (1)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
docs/src/components/common-ui/vben-modal.md (4)
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:31-31
Timestamp: 2024-12-04T04:41:30.161Z
Learning: 在 Vben Admin 项目的 `Page` 组件中，`extra` 内容仅通过 slots 提供，没有对应的 prop 属性。
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:23-24
Timestamp: 2024-12-04T04:43:22.179Z
Learning: 在 `docs/src/components/layout-ui/page.md` 的 `Page` 组件中，`title` 和 `description` 属性有意使用 `string|slot` 类型表示法，表示它们可以接受字符串或插槽。
⏰ 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). (8)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (10)
packages/@core/ui-kit/popup-ui/src/modal/modal.ts (1)
8-12: LGTM! Clean interface addition for animation type support.The
animationTypeproperty is properly defined with appropriate type constraints, default value documentation, and consistent Chinese comments.packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (1)
62-62: LGTM! Proper default state initialization.The
animationType: 'slide'default value is correctly added to thedefaultStateobject, ensuring consistent behavior when no animation type is specified.docs/src/components/common-ui/vben-modal.md (2)
59-66: LGTM! Comprehensive documentation for the new animation feature.The documentation clearly explains both animation types with appropriate descriptions and includes a demo preview for visual demonstration.
124-124: LGTM! Proper props table documentation.The
animationTypeproperty is correctly documented in the props table with accurate type information and default value.packages/@core/ui-kit/popup-ui/src/modal/modal.vue (2)
97-97: LGTM! Proper prop extraction from priority values.The
animationTypeis correctly extracted fromusePriorityValuesfollowing the established pattern for other modal properties.
248-248: LGTM! Clean prop forwarding to DialogContent.The
animationTypeis properly forwarded to theDialogContentcomponent using the correct kebab-case attribute name.docs/src/demos/vben-modal/animation-type/index.vue (2)
1-19: LGTM! Well-structured demo component setup.The component properly demonstrates both animation types using separate modal instances with clear, descriptive function names and proper Vue 3 composition API usage.
21-36: LGTM! Clean template implementation with effective demonstration.The template provides a clear visual demonstration of both animation types with descriptive Chinese labels and well-organized layout using appropriate spacing classes.
packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue (2)
23-23: LGTM: Well-typed animation prop with sensible default.The
animationTypeprop is properly typed with a union type and has a sensible default value. This approach prevents breaking changes while enabling the new animation feature.Also applies to: 37-37
52-52: LGTM: Correct prop exclusion from delegation.The
animationTypeprop is properly excluded from delegated props, preventing it from being passed to the underlying radix-vue DialogContent component. This is the correct approach for internal props.
| 'z-popup bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 w-full p-6 shadow-lg outline-none sm:rounded-xl', | ||
| { | ||
| 'data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-top-[48%]': | ||
| animationType === 'slide', | ||
| }, |
There was a problem hiding this comment.
Fix conflicting animation classes.
The current implementation applies zoom animation classes unconditionally while only conditionally applying slide classes. This creates conflicting animations when animationType is 'slide', as both zoom and slide animations will be active simultaneously.
- 'z-popup bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 w-full p-6 shadow-lg outline-none sm:rounded-xl',
+ 'z-popup bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 w-full p-6 shadow-lg outline-none sm:rounded-xl',
{
+ 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95':
+ animationType === 'scale',
'data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-top-[48%]':
animationType === 'slide',
},📝 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.
| 'z-popup bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 w-full p-6 shadow-lg outline-none sm:rounded-xl', | |
| { | |
| 'data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-top-[48%]': | |
| animationType === 'slide', | |
| }, | |
| 'z-popup bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 w-full p-6 shadow-lg outline-none sm:rounded-xl', | |
| { | |
| 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95': | |
| animationType === 'scale', | |
| 'data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-top-[48%]': | |
| animationType === 'slide', | |
| }, |
🤖 Prompt for AI Agents
In packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue around lines
110 to 114, the zoom animation classes are applied unconditionally, causing
conflicts when animationType is 'slide' because slide animation classes are
conditionally applied. To fix this, make the zoom animation classes conditional
so they only apply when animationType is not 'slide', ensuring only one
animation type is active at a time.
There was a problem hiding this comment.
我并不需要加 ai 提示的这段话，scale 时候没有额外的 class 本来就是走 dialog 自带的效果，以后如果有其他动画，加在这里也不会影响 scale。
There was a problem hiding this comment.
@Elm1992 理解了！您说得对，scale 动画确实应该使用 dialog 自带的 zoom 效果作为基础动画。当前的实现是有意为之的：
scale动画：使用基础的zoom-out-95和zoom-in-95类（始终存在）slide动画：在基础 zoom 效果之上叠加 slide 类来实现滑动效果- 未来的动画类型：可以类似地在基础效果上添加条件类
这种设计模式确实很合理，基础动画效果作为默认行为，特定动画类型通过添加额外的条件类来增强效果。感谢您的解释！
(\ /)
( ._.)
o_(")(") *nods in understanding*
✏️ Learnings added
Learnt from: Elm1992
PR: vbenjs/vue-vben-admin#6537
File: packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue:110-114
Timestamp: 2025-07-17T16:38:41.877Z
Learning: In the Vue-Vben-Admin DialogContent component, the animation system uses a layered approach: zoom animation classes (zoom-out-95, zoom-in-95) are applied unconditionally as the base effect for scale animation, while specific animation types like 'slide' add conditional classes on top of the base effect. This design allows scale animation to use the dialog's built-in zoom effect and enables future animation types to be added without affecting the scale behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
2 participants
Description
想用 radix-vue Dialog 自带的效果， 万一拓展新的动画效果用 animationType 也算是留个接口。
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Summary by CodeRabbit
New Features
Documentation