Repository navigation
fix: the bug in the lock method of the vbenModal component - #6648
Conversation
|
WalkthroughAdjusted modal state management in modal-api.ts: opening the modal now explicitly resets submitting to false, while closing no longer modifies submitting and only sets isOpen to false. No public API signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ModalAPI
participant ModalState
Caller->>ModalAPI: open()
ModalAPI->>ModalState: set { isOpen: true, submitting: false }
Caller->>ModalAPI: close()
ModalAPI->>ModalState: set { isOpen: false }
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (2)
packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (2)
38-63: Initialize submitting in defaultState to stabilize reactive shapesubmitting is read and written elsewhere (lock/open), but it isn’t present in the initial state. Defining it up-front avoids undefined vs false checks and ensures a stable object shape for reactivity and SSR hydration.
Apply this diff to add it:
confirmLoading: false, + submitting: false, contentClass: '',
150-152: Optional guard: ignore confirm while locked or not openThis provides a cheap extra safety net against stray events during closing/locked states. Not required for the main fix, but it hardens the API.
Apply this diff:
- onConfirm() { - this.api.onConfirm?.(); - } + onConfirm() { + // Ignore redundant confirms while locked or while modal is not open + if (this.state?.submitting || !this.state?.isOpen) return; + this.api.onConfirm?.(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts(1 hunks)
⏰ 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). (7)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (2)
packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (2)
164-168: Moving submitting reset to open() fixes the post-close double-click race — LGTMClearing submitting only on open prevents the transient re-enable window during close. This directly addresses the reported double-confirm issue from rapid clicks.
102-112: Confirm no internal breakages, but audit external usages of submitting flag
We’ve verified that inmodal-api.tsthe newclose()no longer clearssubmitting, whereasopen()still setssubmitting: false. Internally, the only logic tied tosubmittingon close is the gating on update:open (@update:open="() => (!submitting ? modalApi.close() : undefined)") and the usual escape/interact blockers inmodal.vue. No other handlers (includinghandleClosed) resetsubmitting.However, since drawer’s
close()still resetssubmittingand external consumers might rely onmodalApi.onClosedor watchers seeingsubmittingclear when a modal closes, please:• Audit any downstream code (in applications or other packages) that:
– CallsmodalApi.close()and then readssubmitting
– Relies ononClosedcallbacks to run withsubmitting === false
– Uses@update:openor other transition hooks expecting submitting to reset on close
• Consider either restoringthis.store.setState({ submitting: false })inmodal-api.ts#close()for parity with drawer, or explicitly documenting thatsubmittingnow persists through close until the nextopen()No direct internal fixes needed, but this is a breaking behavior change for consumers—please verify and update accordingly.
Description
vbenModal 组件，设置 lock 方法，然后 close 方法关闭弹窗，快速点击有时候执行了两次确定按钮。
简单的 modal close 方法加个定时器快速点击偶尔也能复现
项目里 modal 实际 dom 会多一些，快速点击很容易复现，modal 消失之前及时是有 lock 方法，页面关闭之前也能再点击一次。
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Summary by CodeRabbit