perf: add nested modal demo #5353
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
playground/src/views/examples/modal/index.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 new nested modal functionality in the Vben Admin V5 project. It modifies the modal examples in Changes
Sequence DiagramsequenceDiagram
participant User
participant MainModal
participant NestedModal
User->>MainModal: Open Main Modal
MainModal->>User: Display Main Modal
User->>MainModal: Click Open Nested Modal
MainModal->>NestedModal: Trigger Nested Modal
NestedModal->>User: Display Nested Modal
Assessment against linked issues
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
playground/src/views/examples/modal/nested-demo.vue (3)
1-7: Consider adding component documentation.Since this is a new demo component, adding JSDoc comments would help other developers understand its purpose and usage.
Add documentation above the script tag:
+/** + * Nested Modal Demo + * + * Demonstrates the ability to open a modal within another modal using VbenModal. + * The outer modal contains a button that triggers an inner modal using DragDemo component. + */ <script lang="ts" setup>
8-13: Consider adding type annotations for modal instances.The modal instances could benefit from explicit type annotations for better type safety and documentation.
-const [Modal] = useVbenModal({ +const [Modal] = useVbenModal<void>({ destroyOnClose: true, }); -const [BaseModal, baseModalApi] = useVbenModal({ +const [BaseModal, baseModalApi] = useVbenModal<void>({ connectedComponent: DragDemo, });
20-22: Consider translating UI text to English.The modal title and button text are in Chinese. For consistency and international usage, consider using English or implementing i18n.
- <Modal title="嵌套弹窗示例"> - <Button @click="openNestedModal" type="primary">打开子弹窗</Button> + <Modal title="Nested Modal Example"> + <Button @click="openNestedModal" type="primary">Open Child Modal</Button>playground/src/views/examples/modal/index.vue (2)
46-48: Consider adding type annotation for nested modal.For consistency with the suggestion in nested-demo.vue, consider adding type annotation here as well.
-const [NestedModal, nestedModalApi] = useVbenModal({ +const [NestedModal, nestedModalApi] = useVbenModal<void>({ connectedComponent: NestedDemo, });
173-178: Consider translating card text to English.Similar to the nested-demo.vue suggestion, consider translating the card title and description to English or implementing i18n.
- <Card class="w-[300px]" title="嵌套弹窗示例"> - <p>在已经打开的弹窗中再次打开弹窗</p> + <Card class="w-[300px]" title="Nested Modal Example"> + <p>Open a modal within an already opened modal</p> <template #actions> - <Button type="primary" @click="openNestedModal">打开嵌套弹窗</Button> + <Button type="primary" @click="openNestedModal">Open Nested Modal</Button> </template> </Card>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
playground/src/views/examples/modal/index.vue(5 hunks)playground/src/views/examples/modal/nested-demo.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (2)
playground/src/views/examples/modal/index.vue (2)
4-4: LGTM! Clean import additions.The imports for new components (Flex, Card) and NestedDemo are properly organized.
Also applies to: 13-13
114-179: Verify accessibility of the new card layout.The new Flex and Card layout looks clean, but ensure it's accessible:
- Check if the layout works well with screen readers
- Verify keyboard navigation between cards
- Ensure sufficient color contrast
Description
添加嵌套弹窗演示。
close #5352
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
New Features
Improvements