-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
Update modal-api.ts #5399
Update modal-api.ts #5399
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/modal/modal-api.tsOops! 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 method Changes
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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: 0
🧹 Nitpick comments (1)
packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (1)
112-114
: Enhance method documentation using JSDoc format.The current documentation should be improved to:
- Use English to maintain consistency with the codebase
- Include parameter and return value descriptions
- Follow JSDoc format for better IDE integration
- /** - * 提交时等待 禁用相关操作 - */ + /** + * Disables modal interactions and shows loading state during async operations. + * @param flag - When true, enables loading state and disables interactions. + * When false, restores normal modal behavior. + * @returns The modal instance for method chaining. + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 (9)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (3)
packages/@core/ui-kit/popup-ui/src/modal/modal-api.ts (3)
115-123
: LGTM! Implementation effectively handles modal state during submissions.The implementation efficiently updates multiple modal properties in a single setState call, correctly toggling both loading indicators and interaction controls.
124-124
: LGTM! Correct implementation of method chaining.The return value maintains consistency with other chainable methods in the class.
115-125
: Verify interaction with onConfirm handling.While the implementation is clean, please verify that rapid toggling of loadingAndDisable doesn't interfere with onConfirm handling, especially in cases where the confirmation process takes time to complete.
✅ Verification successful
Implementation is safe for async operations
The implementation of
loadingAndDisable
is well-designed to handle async operations inonConfirm
. It properly disables all closing mechanisms (escape key, overlay clicks, close button) and sets loading states atomically, preventing race conditions during async operations.Example usage pattern found in form-modal-demo.vue demonstrates the intended async flow with form validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of loadingAndDisable in conjunction with onConfirm # to ensure proper handling of async confirmation flows rg -l "loadingAndDisable.*onConfirm|onConfirm.*loadingAndDisable"Length of output: 67
Script:
#!/bin/bash # Find the complete ModalApi implementation ast-grep --pattern 'class ModalApi { $$$ }' # Search for onConfirm usage patterns rg "onConfirm.*=.*" -A 3 # Look for loadingAndDisable usage rg "loadingAndDisable\(" -A 2Length of output: 15906
增加 提交时禁用相关关闭操作
Description
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