fix(dialog-select): fixed an issue where grid data could not be loaded when initially visible was set to true #2835
Conversation
…d when initially visible was set to true
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
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 (
|
WalkthroughThis pull request addresses a bug in the dialog-select component where grid data could not be loaded when the component was initially set to visible. The change ensures that the grid data is queried immediately when the visibility is set to true. Changes
|
| } | ||
| }) | ||
| }, | ||
| { immediate: true } |
There was a problem hiding this comment.
The addition of { immediate: true } ensures that the watcher triggers immediately if the visible prop is true at the time of component creation. This is a crucial fix for ensuring that grid data is loaded as expected when the dialog is initially visible.
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/renderless/src/dialog-select/vue.ts (1)
133-139: LGTM! The changes effectively address the initial visibility issue.The modifications to the watcher are well-implemented:
- Using
nextTickensures proper timing of the data fetch after DOM updates- Adding
immediate: trueensures the watcher triggers on component creation, fixing the initial visibility caseSuggestion: Consider adding error handling.
While the implementation is correct, consider adding error handling for the data fetching operation.
watch( () => props.visible, (value) => { nextTick(() => { if (value && !state.multiGridStore.inited) { - api.queryGridData() + api.queryGridData().catch(error => { + console.error('Failed to load grid data:', error) + // Consider emitting an error event or updating state to show error UI + }) } }) }, { immediate: true } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/renderless/src/dialog-select/vue.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Unit Test
🔇 Additional comments (1)
packages/renderless/src/dialog-select/vue.ts (1)
133-139: Verify loading state handling during data fetch.Please ensure that:
- The loading state (
state.multiGridStore.loading) is properly managed during the data fetch- The component handles race conditions if visibility changes while data is being fetched
Let's verify the implementation of
queryGridData:✅ Verification successful
Loading state is properly managed ✅
The implementation correctly handles loading states:
- Sets loading flag at the start of data fetch
- Clears loading state in all execution paths (success/error)
- Prevents race conditions using the
initedflag- UI properly consumes the loading state for feedback
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for queryGridData implementation to verify loading state management ast-grep --pattern 'queryGridData = $_({ $$$, state, $$ }) => { $$$ }'Length of output: 74
Script:
#!/bin/bash # Try different patterns to locate queryGridData echo "=== Searching for queryGridData implementation ===" ast-grep --pattern 'queryGridData() { $$ }' ast-grep --pattern 'queryGridData: function() { $$ }' ast-grep --pattern 'const queryGridData = async $_' echo -e "\n=== Searching for loading state management ===" rg "multiGridStore.*loading" -A 3Length of output: 2069
…d when initially visible was set to true
PR
修复dialog-select组件，在初始设置visible为true时， 不加载grid数据的问题
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit