-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
fix: change drawer
and modal
zIndex to 2000 compatible with element-plus
#5293
base: main
Are you sure you want to change the base?
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vueOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request introduces a consistent update to the Changes
Suggested labels
Suggested reviewers
Poem
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 (
|
drawer
and modal
zIndex to 2000 compatible with element-plus
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 (2)
docs/src/components/common-ui/vben-drawer.md (1)
103-103
: LGTM! Consider adding a compatibility note.The z-index update to 2000 aligns well with element-plus's z-index system.
Consider adding a note about element-plus compatibility, like:
| zIndex | 抽屉的ZIndex层级 | `number` | `2000` | + +> Note: The default z-index of 2000 is chosen for compatibility with element-plus components.docs/src/components/common-ui/vben-modal.md (1)
113-113
: LGTM! Consider adding a compatibility note.The z-index update to 2000 maintains consistency with the drawer component and aligns with element-plus's z-index system.
Consider adding a note about element-plus compatibility, like:
| zIndex | 弹窗的ZIndex层级 | `number` | `2000` | + +> Note: The default z-index of 2000 is chosen for compatibility with element-plus components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/drawer.vue
(1 hunks)packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue
(1 hunks)packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogScrollContent.vue
(1 hunks)packages/@core/ui-kit/shadcn-ui/src/ui/sheet/SheetContent.vue
(1 hunks)
🔇 Additional comments (5)
packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogScrollContent.vue (1)
19-19
: Consistent zIndex update.Raising the default
zIndex
from 1000 to 2000 helps align with element-plus components. This looks good.packages/@core/ui-kit/shadcn-ui/src/ui/sheet/SheetContent.vue (1)
30-30
: Aligning zIndex with external library requirements.The default
zIndex
value is now set to 2000, which is in line with the element-plus library. This standardizes component layering and avoids overlap issues.packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogContent.vue (1)
32-32
: Ensuring visibility above element-plus components.The updated default
zIndex
value of 2000 is consistent with other components, preventing stacking conflicts.packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (1)
39-39
: UnifiedzIndex
approach for drawer overlay.Updating the drawer default
zIndex
to 2000 aligns this component with the new standard across the codebase, preventing display conflicts when used alongside element-plus components.docs/src/components/common-ui/vben-modal.md (1)
Line range hint
103-103
: Verify z-index consistency across all overlay components.The z-index updates in both drawer and modal components look good. Let's verify that all related overlay components have been updated consistently.
Also applies to: 113-113
✅ Verification successful
Z-index values are consistent across overlay components
Based on the code inspection:
- The only high z-index values (9999) found are in loading screens, which is appropriate for app-level overlays
- The modal component accepts a configurable z-index prop
- The drawer component has a default z-index of 2000
- The dialog and sheet overlays use CSS classes for layering without hardcoded z-index values
- No components were found with the problematic z-index value of 1000
The z-index implementation across overlay components appears to be consistent and follows good practices by:
- Using configurable z-index props where needed
- Leveraging CSS classes for layering
- Maintaining appropriate stacking contexts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining z-index values that might need updating # Look for z-index values in vue files that might still be set to 1000 # Search for z-index declarations with value 1000 rg -i "z-?index.*1000" --type vue # Search for style declarations with z-index: 1000 rg "z-index:\s*1000" --type vueLength of output: 125
Script:
#!/bin/bash # Search for z-index declarations in all files, not limiting to specific file types rg -i "z-?index.*1000" # Search for style declarations with z-index: 1000 rg "z-index:\s*1000" # Also search for other common z-index values used in overlays (e.g., 999, 1001, etc.) rg -i "z-?index.*(999|1001)" # Search for files that might contain overlay components fd -e vue -e tsx -e jsx -e css -e scss -e less "dialog|modal|drawer|popup|overlay"Length of output: 1903
Script:
#!/bin/bash # Let's check the z-index values in the overlay components we found # Focusing on the core UI components first # Check DialogOverlay.vue content cat "packages/@core/ui-kit/shadcn-ui/src/ui/dialog/DialogOverlay.vue" # Check SheetOverlay.vue content cat "packages/@core/ui-kit/shadcn-ui/src/ui/sheet/SheetOverlay.vue" # Check modal.vue content cat "packages/@core/ui-kit/popup-ui/src/modal/modal.vue" # Check drawer.vue content cat "packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue"Length of output: 16100
Description
Why?
element-plus some pop-up controls have a z-index of 2000, causing some display issues.
https://github.com/search?q=repo%3Aelement-plus%2Felement-plus%20z-index%3A%202000&type=code
Before
After
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
Documentation
Chores
zIndex
from1000
to2000
across multiple UI components