-
Notifications
You must be signed in to change notification settings - Fork 8.4k
fix: vbenTree modelValue synchronization #5825
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
Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request simplifies the model handling logic in the tree component. In Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TreeComponent
User->>TreeComponent: Initiates selection event
TreeComponent->>TreeComponent: Calls event.preventDefault()
TreeComponent->>TreeComponent: Executes onSelect()
TreeComponent->>TreeComponent: Invokes updateTreeValue() (checks for undefined and maps values)
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🪧 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 (2)
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue (2)
269-270: Consider cleaning up commented code in templateThere are several commented lines in the event handling section that appear to be leftover from previous implementations. Consider removing these comments to improve code readability and maintenance.
@click=" () => { handleSelect(); - // onSelect(item, !isSelected); } " /> <div class="flex items-center gap-1 pl-2" @click=" (_event) => { - // $event.stopPropagation(); - // $event.preventDefault(); handleSelect(); - // onSelect(item, !isSelected); } "Also applies to: 277-281
155-176: Consider simplifying the onSelect function furtherWhile the PR addresses the main synchronization issue, the onSelect function still contains complex logic for parent-child relationships. Since you've moved to the defineModel pattern, consider further simplifying this function and leveraging more of radix-vue's built-in functionality for propagation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue(3 hunks)packages/@core/ui-kit/shadcn-ui/src/ui/tree/types.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/@core/ui-kit/shadcn-ui/src/ui/tree/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (3)
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue (3)
76-76: Good use of defineModel for simplified two-way bindingReplacing useVModel with Vue 3's defineModel is a cleaner approach for handling two-way binding. This change eliminates the need for manual deep watching and default value assignment, which were likely sources of the synchronization issues.
101-107: Proper handling of undefined modelValue casesThe addition of an explicit check for undefined modelValue values ensures the component behaves correctly in edge cases, such as when no initial value is provided or when the value is cleared. This fix directly addresses potential synchronization issues when the value is reset or undefined.
233-233: Event propagation prevention is now activeUncommenting the event.preventDefault() call ensures that native browser events don't interfere with the custom selection logic, preventing potential issues with event bubbling that could affect selection behavior.
Description
修复VbenTree的值未能正确同步的问题
fixed: #5822
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
modelValueproperty, enhancing overall user experience with a more straightforward and predictable selection process.