-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
feat: modal state locked on submitting #5401
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces enhancements to the modal component across multiple files, focusing on improving state management and submission handling. The changes include adding a new Changes
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 (
|
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: 2
🧹 Nitpick comments (4)
playground/src/views/examples/modal/index.vue (1)
99-101
: Document the modal locking feature in the demo.Since this PR introduces modal locking during submission, consider adding a comment or description in the form modal demo section to highlight this new feature. This will help users understand the locking behavior during form submission.
Add a description in the card component:
<Card class="w-[300px]" title="表单弹窗示例"> - <p>弹窗与表单结合</p> + <p>弹窗与表单结合,展示了提交过程中的锁定机制</p> <template #actions>packages/@core/ui-kit/popup-ui/src/modal/modal.ts (1)
118-121
: Enhance submitting prop documentation.Consider adding default value and usage example to the documentation.
/** * 提交中(锁定弹窗状态) + * @default false + * @example + * // Lock modal during form submission + * modal.setState({ submitting: true }); */ submitting?: boolean;packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (1)
117-124
: Add validation to lock method.The lock method should validate the modal's current state.
lock(isLocked = true) { + if (this.state.submitting === isLocked) { + return this; // Already in desired state + } return this.setState({ submitting: isLocked }); }packages/@core/ui-kit/popup-ui/src/modal/modal.vue (1)
182-182
: Consider improving readability of the update handler.While functionally correct, the expression could be more readable.
- @update:open="() => (!submitting ? modalApi?.close() : undefined)" + @update:open="() => { + if (!submitting) { + modalApi?.close(); + } + }"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/src/components/common-ui/vben-modal.md
(2 hunks)packages/@core/base/typings/src/helper.d.ts
(2 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts
(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal.ts
(3 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal.vue
(9 hunks)packages/styles/src/antd/index.css
(0 hunks)playground/src/views/examples/modal/form-modal-demo.vue
(1 hunks)playground/src/views/examples/modal/index.vue
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/styles/src/antd/index.css
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (9)
playground/src/views/examples/modal/index.vue (1)
100-100
: Verify form field consistency across components.The addition of
field2: '123'
to the form values suggests that the corresponding form component (FormModalDemo
) expects this new field. Please ensure that:
- The form component (
form-modal-demo.vue
) has been updated to handle this new field- The form validation, if any, has been updated accordingly
Let's verify the form field usage:
✅ Verification successful
Form field consistency verified successfully
The
field2
field is properly defined inform-modal-demo.vue
with required validation and appropriate labeling, maintaining consistency with the initial values provided inindex.vue
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for field2 usage in form-modal-demo.vue and related files rg "field2" --type vueLength of output: 50
Script:
#!/bin/bash # Search for field2 usage with context rg -C 3 "field2" playground/src/views/examples/modal/ # Also search for form-modal-demo component definition fd "form-modal-demo" playground/src/views/examples/modal/Length of output: 1137
packages/@core/base/typings/src/helper.d.ts (1)
112-112
: LGTM! Well-defined type for async operations.The
MaybePromise<T>
type elegantly handles both synchronous and asynchronous operations.packages/@core/ui-kit/popup-ui/src/modal/modal.ts (1)
164-164
: LGTM! Improved type safety for close operations.The use of
MaybePromise
allows for both sync and async close operations.packages/@core/ui-kit/popup-ui/src/modal/modal.vue (5)
83-83
: LGTM! Well-implemented loading state handling.The watch function elegantly handles both
showLoading
andsubmitting
states, ensuring consistent UX by scrolling to top when either state changes.Also applies to: 119-121
139-141
: LGTM! Comprehensive modal interaction handling during submission.The implementation consistently prevents all forms of modal dismissal (click outside, escape key, pointer events) when in submitting state, ensuring data integrity during form submission.
Also applies to: 145-147, 160-164
203-203
: LGTM! Robust content protection during submission.The implementation effectively prevents content interaction during submission by:
- Disabling the close button
- Adding pointer-events-none to prevent content interaction
Also applies to: 255-255
260-262
: LGTM! Consistent loading state visualization.The loading indicator correctly responds to both
showLoading
andsubmitting
states, providing clear visual feedback to users.
295-295
: LGTM! Comprehensive button state management.The implementation effectively prevents multiple submissions by:
- Disabling the cancel button during submission
- Showing loading state on the confirm button
Also applies to: 307-307
docs/src/components/common-ui/vben-modal.md (1)
116-116
: LGTM! Clear and comprehensive documentation.The documentation effectively explains:
- The purpose and default value of the
submitting
property- The behavior and effects of the
lock
method- The user experience changes during locked state
Also applies to: 157-163
Description
为弹窗添加lock方法和submitting属性用于锁定提交状态,
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
Release Notes
New Features
submitting
state.Improvements
Documentation