-
Notifications
You must be signed in to change notification settings - Fork 8.4k
fix: tree component cannot set a value #6941
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
Signed-off-by: aonoa <1991849113@qq.com>
|
WalkthroughThe Tree component's checkbox binding mechanism is updated from using the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
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/shadcn-ui/src/ui/tree/tree.vue (1)
307-314: "Select all" checkbox lacks visual state binding.The checkbox has an
@update:model-valuehandler but no:model-valueprop to display its current state. Users cannot see whether all items are selected or not.Consider adding a computed property to track the "all selected" state and bind it to the checkbox:
<Checkbox v-if="multiple" + :model-value=" + treeValue?.length > 0 && + treeValue?.length === flattenData.filter((item) => !get(item.value, disabledField)).length + " @click.stop @update:model-value=" (checked: boolean | 'indeterminate') => checked === true ? checkAll() : unCheckAll() " />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-09T04:41:58.914Z
Learnt from: mynetfan
Repo: vbenjs/vue-vben-admin PR: 5075
File: packages/effects/common-ui/src/components/api-select/api-select.vue:61-62
Timestamp: 2024-12-09T04:41:58.914Z
Learning: 在文件 `packages/effects/common-ui/src/components/api-select/api-select.vue` 的组件 `ApiSelect` 中,`childrenField` 的默认值设置为空字符串是有意的,因为并非所有组件都使用树形数据。
Applied to files:
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue
⏰ 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 (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (1)
packages/@core/ui-kit/shadcn-ui/src/ui/tree/tree.vue (1)
383-383: Critical fix correctly addresses the selection binding issue.Changing from
:checkedto:model-valueis essential for Vue 3 two-way binding with the Checkbox component. This resolves the reported issues where default selection couldn't be set and parent-child linkage was broken.
|
Please accept and merge. thanks |
Description
fixed: #6906
-->
Type of change
Please delete options that are not relevant.
Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit