-
Notifications
You must be signed in to change notification settings - Fork 197
[WIP]fix:parent node not disabled when child nodes count greater than maxCount
#642
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改针对 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OptionList
participant TreeEntity
User->>OptionList: 调用 getDisabledWithCache(entity)
OptionList->>OptionList: 使用栈进行深度优先遍历子节点
OptionList->>TreeEntity: 访问每个子节点,判断是否叶子且可勾选
OptionList-->>OptionList: 计数,超过 leftMaxCount 则提前终止
OptionList-->>User: 返回禁用状态缓存结果
Possibly related PRs
Suggested reviewers
Poem
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
examples/mutiple-with-maxCount.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. src/OptionList.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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:
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)
src/OptionList.tsx (1)
181-205
: 递归计数函数存在重复创建与深度遍历性能问题,可考虑优化
getLeafCheckableCount
定义在getDisabledWithCache
内部,每次计算都会重新创建函数;在大树和频繁调用场景(如滚动虚拟列表)中会带来不必要的 GC 压力。可将其提到外层并以参数注入所需上下文,或使用useCallback
缓存。- 当前递归会完整遍历子树,直到所有叶子统计完毕。若叶子数一旦超过
leftMaxCount
,后续遍历已无意义,可及时return count
或抛出标记提前终止,提高效率。- 对极深层树结构,JS 递归有栈溢出的风险(> ~10k 递归深度);可以改用显式栈迭代保证安全。
示例改动(仅演示早停逻辑):
const getLeafCheckableCount = ( - children: DataEntity<DataNode>[], -): number => { + children: DataEntity<DataNode>[], + limit: number, +): number => { return children.reduce((count, current) => { + if (count > limit) { // 早停 + return count; + } ... - return count + getLeafCheckableCount(currentEntity.children); + return count + getLeafCheckableCount(currentEntity.children, limit - count); }, 0); }; -const checkableChildrenCount = getLeafCheckableCount(entity.children); +const checkableChildrenCount = getLeafCheckableCount(entity.children, leftMaxCount);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/OptionList.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/OptionList.tsx (1)
src/interface.ts (1)
DataNode
(6-10)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (1)
src/OptionList.tsx (1)
168-173
: 缓存刷新条件可能遗漏checkedKeys
变化
disabledCache
只在leftMaxCount
变化时清空。如果leftMaxCount
的计算并未随每一次勾选/取消勾选同步更新(取决于上层实现),那么在同一个leftMaxCount
周期内用户改变勾选状态后,缓存的可选状态可能过期,导致父节点禁用/启用状态与实际剩余可选数量不一致。建议确认:
leftMaxCount
是否一定随checkedKeys
的变化而变化。- 如不确定,可把
checkedKeys
也加入依赖,或在勾选操作后调用setDisabledCache(new Map())
以保证一致性。
src/OptionList.tsx
Outdated
); | ||
|
||
const checkableChildrenCount = checkableChildren.length; | ||
const getLeafCheckableCount = (children: DataEntity<DataNode>[]): number => { |
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.
这样会有性能问题,最好是收集可见区域的节点,通过染色来做标记。避免递归产生大量的性能损耗。
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.
那是不是可以通过栈+染色的形式来进行一个优化
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)
examples/mutiple-with-maxCount.tsx (1)
67-69
: onCheckChange 函数在当前示例中未被使用
onCheckChange
函数和checkValue
状态在当前简化后的UI中似乎没有被使用到。由于只保留了一个 TreeSelect 组件,且该组件仅使用了onChange
和value
属性,可以考虑移除这些未使用的代码。- const [checkValue, setCheckValue] = useState<string[]>(); - const onCheckChange = (val: string[]) => { - setCheckValue(val); - };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/mutiple-with-maxCount.tsx
(2 hunks)src/OptionList.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/OptionList.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (3)
examples/mutiple-with-maxCount.tsx (3)
5-6
: 将初始状态从预设值改为 undefined 是合理的调整这里的修改将
value
和checkValue
的初始状态从预设数组改为undefined
,这样使得树选择组件初始时不预选任何节点,符合测试案例的需求。
24-43
: 扩展树结构深度有助于更好地测试问题场景添加更深层次的树结构非常有利于测试当子节点数量超过
maxCount
时父节点是否正确禁用的问题。这个5层深的结构能够充分验证PR中修复的功能。
77-80
: 明确设置 multiple 和 maxCount 属性有助于突显问题添加
multiple
属性并明确设置maxCount={3}
使得示例更加聚焦于要解决的问题,有利于验证当子节点总数超过3个时父节点是否能正确禁用。
const currentEntity = stack.pop(); | ||
const currentValue = currentEntity.node[fieldNames.value]; | ||
|
||
if (visited.has(currentValue)) { |
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.
这个 visited 是在 getDisabledWithCache 里申明的,那每次进来都会是一个新的
|
||
if (isCurrentLeaf) { | ||
if (!isDisabled) { | ||
checkableCount++; |
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.
其实深度遍历的话可以考虑直接做个 dig function,然后计数器超过 maxCount 直接短路就行了。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #642 +/- ##
==========================================
- Coverage 99.83% 99.50% -0.33%
==========================================
Files 16 16
Lines 593 611 +18
Branches 186 180 -6
==========================================
+ Hits 592 608 +16
- Misses 1 3 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Related issue #53567
Before:

之前的计算逻辑是只判断当前节点的直接子节点是否disabled,然后计算其总数
导致计算出来的title为'1'对应的node未被正确的赋上disabled属性
2025-04-22.064749.mp4
After:
将未禁用状态下的非叶子节点的子节点也进行统计、计算
2025-04-22.162816.mp4
Summary by CodeRabbit