feat: support set default props for drawer and modal #5390
Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces new functionality to set default properties for modal and drawer components across multiple applications. The changes involve adding Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Nitpick comments (3)
apps/web-ele/src/bootstrap.ts (1)
26-34: Consider centralizing z-index management.While the implementation is correct, managing z-index values across different applications can become maintenance overhead. Consider:
- Creating a shared configuration for z-index values
- Using CSS custom properties (variables) for z-index management
- Documenting the z-index hierarchy in the codebase
packages/styles/src/antd/index.css (1)
58-60: LGTM! Consider documenting z-index layers.The z-index value of 1050 for
.ant-messageis appropriately chosen to ensure messages appear above regular components while working with maximized vxeTable.Consider maintaining a documentation of z-index layers in a constants file or documentation to help track the visual hierarchy of components:
// src/constants/zIndex.ts export const Z_INDEX = { MESSAGE: 1050, MODAL: 1000, // ... other z-index values } as const;docs/src/components/common-ui/vben-drawer.md (1)
58-58: Enhance the documentation with an example.The documentation addition is helpful but could be more effective with a concrete example.
Consider adding a code example:
- 如果抽屉的默认行为不符合你的预期，可以在`src\bootstrap.ts`中修改`setDefaultDrawerProps`的参数来设置默认的属性，如默认隐藏全屏按钮，修改默认ZIndex等。 + 如果抽屉的默认行为不符合你的预期，可以在`src\bootstrap.ts`中修改`setDefaultDrawerProps`的参数来设置默认的属性： + + ```ts + // src/bootstrap.ts + setDefaultDrawerProps({ + fullscreenButton: false, // 默认隐藏全屏按钮 + zIndex: 2000, // 修改默认ZIndex + }); + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web-antd/src/bootstrap.ts(2 hunks)apps/web-ele/src/bootstrap.ts(2 hunks)apps/web-naive/src/bootstrap.ts(2 hunks)docs/src/components/common-ui/vben-drawer.md(1 hunks)docs/src/components/common-ui/vben-modal.md(1 hunks)packages/@core/ui-kit/popup-ui/src/drawer/index.ts(1 hunks)packages/@core/ui-kit/popup-ui/src/drawer/use-drawer.ts(2 hunks)packages/@core/ui-kit/popup-ui/src/modal/index.ts(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/use-modal.ts(2 hunks)packages/styles/src/antd/index.css(1 hunks)playground/src/bootstrap.ts(2 hunks)
🔇 Additional comments (9)
packages/@core/ui-kit/popup-ui/src/modal/index.ts (1)
3-3: LGTM! Clean export addition.The export statement correctly exposes the new
setDefaultModalPropsfunction while maintaining the existinguseVbenModalexport.packages/@core/ui-kit/popup-ui/src/drawer/index.ts (1)
3-3: LGTM! Consistent with modal implementation.The export statement follows the same pattern as the modal implementation, maintaining consistency across components.
apps/web-naive/src/bootstrap.ts (2)
4-8: LGTM! Clean import organization.The imports are well-organized and grouped appropriately from the
@vben/common-uimodule.
26-34: Consider translating Chinese comments to English.While the implementation is correct, consider translating the Chinese comments to English for better maintainability in an international codebase:
- // 设置弹窗的默认配置 + // Set default modal configuration - // 设置抽屉的默认配置 + // Set default drawer configurationAlso, verify that the z-index value of 2000 is appropriate for the Naive UI ecosystem:
✅ Verification successful
Consider translating Chinese comments to English for better maintainability
While the implementation is correct, consider translating the Chinese comments to English:
- // 设置弹窗的默认配置 + // Set default modal configuration - // 设置抽屉的默认配置 + // Set default drawer configurationThe z-index value of 2000 is appropriate as it's consistently used across the codebase for similar UI components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any z-index declarations in Naive UI related files rg -i "z-?index.*2000" --type ts --type vueLength of output: 73
Script:
#!/bin/bash # Search for z-index declarations with value 2000 rg -i "z-?index.*2000" # Search for Naive UI configuration files fd -e ts -e js naive # Search for any z-index related constants or configurations in Naive UI files rg -i "z-?index" -g "*naive*"Length of output: 311
apps/web-antd/src/bootstrap.ts (1)
26-34: Verify z-index alignment with Ant Design's layering system.The z-index value of 1020 seems specific to Ant Design. Please verify that this value aligns with Ant Design's z-index hierarchy to ensure proper layering with other components:
Also, consider translating the Chinese comments to English for consistency:
- // 设置弹窗的默认配置 + // Set default modal configuration - // 设置抽屉的默认配置 + // Set default drawer configuration✅ Verification successful
Z-index value of 1020 is correctly aligned with Ant Design's layering system
The z-index hierarchy in the code follows Ant Design's conventions:
- System messages: 1050
- Modal/Drawer: 1020
- Layout elements: 150-200
This ensures proper component layering and interaction.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Ant Design related z-index declarations rg -i "z-?index.*1020" --type ts --type vue # Look for any Ant Design configuration files that might define z-index values fd -e ts -e js -e json antd.*configLength of output: 113
Script:
#!/bin/bash # Search for z-index declarations in the codebase rg -i "z-?index" -A 2 -B 2 # Search for Modal and Drawer component configurations ast-grep --pattern 'Modal { $$$ }' ast-grep --pattern 'Drawer { $$$ }' # Look for any config files fd -e ts -e js -e json "config"Length of output: 30343
packages/@core/ui-kit/popup-ui/src/drawer/use-drawer.ts (2)
24-28: LGTM! Clean implementation of default drawer props.The implementation follows Vue's reactive patterns and ensures proper type safety.
77-81: Verify prop merging precedence.The order of spread operations ensures correct precedence:
- Default props (lowest priority)
- Injected options
- Direct options (highest priority)
packages/@core/ui-kit/popup-ui/src/modal/use-modal.ts (1)
20-24: LGTM! Consistent implementation with drawer default props.The implementation follows the same pattern as the drawer implementation, ensuring consistency across components.
playground/src/bootstrap.ts (1)
27-35: Verify z-index consistency across applications.The z-index value (1020) differs from other applications (2000). This might lead to inconsistent behavior when these components are used together.
Consider:
- Using a consistent z-index across all applications
- Documenting why different values are needed if this is intentional
Run this script to check z-index usage across the codebase:
✅ Verification successful
Z-index values are framework-specific and intentional
The different z-index values (1020 vs 2000) are intentionally set based on the UI framework being used:
- Ant Design based apps use 1020
- Element Plus and Naive UI based apps use 2000
This is consistent with the framework-specific design patterns and the documentation explicitly supports customizing these values in bootstrap.ts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check z-index values used across bootstrap files echo "Checking z-index values in bootstrap files:" rg -A 2 "setDefaultModalProps|setDefaultDrawerProps" | rg "zIndex:"Length of output: 550
Script:
#!/bin/bash # Check framework dependencies and potential documentation echo "Checking package.json files for UI framework versions:" fd package.json --exec grep -l "ant-design\|naive-ui\|element-plus" {} \; --exec cat {} echo -e "\nChecking for z-index related documentation:" rg -i "z-index|zindex" -g "README.md" -g "*.md"Length of output: 7802
Description
添加为弹窗和抽屉设置默认属性的方法。这些方法可以用于定制抽屉、弹窗的统一的默认表现，如不显示全屏按钮、默认可拖动等等。
不同的组件库的默认弹窗层级不同，因此将弹窗和抽屉的默认层级分别设置为各自组件库的默认弹窗层级，而antd的默认层级取了一合适的值，使得其在vxeTable最大化状态下能正常显示且不会遮挡全局提示（message）
close #5387
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
New Features
Documentation
Style
.ant-messageCSS rule with z-index of 1050 for consistent message positioning.These changes provide more flexibility in configuring UI components and improve overall user interface consistency.