-
Notifications
You must be signed in to change notification settings - Fork 491
feat: add disabledStrategy
prop to support dynamic node disabled state calculation
#890
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改主要集中在 Changes
Possibly related PRs
Suggested reviewers
Poem
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: 1
🧹 Outside diff range and nitpick comments (3)
src/contextTypes.ts (1)
73-73
: 代码变更看起来不错,建议添加详细注释新增的
disabledStrategy
属性类型定义清晰且合理。建议为该属性添加 JSDoc 注释,说明其用途和预期行为。建议添加如下注释:
+ /** + * 自定义节点禁用状态的计算策略 + * @param node - 要计算禁用状态的节点 + * @returns 如果节点应该被禁用则返回 true,否则返回 false + */ disabledStrategy?: (node: DataNode) => boolean;src/TreeNode.tsx (1)
234-239
: 建议优化isDisabled
方法的实现建议对该方法进行以下改进:
- 使用可选链操作符来简化代码
- 添加 JSDoc 文档说明该方法的功能和参数
+ /** + * 判断节点是否被禁用 + * @returns {boolean} 如果节点被禁用则返回 true,否则返回 false + */ isDisabled = () => { const { disabled, data } = this.props; const { context: { disabled: treeDisabled, disabledStrategy }, } = this.props; - return !!(treeDisabled || disabled || (disabledStrategy && disabledStrategy(data))); + return !!(treeDisabled || disabled || disabledStrategy?.(data)); };🧰 Tools
🪛 Biome (1.9.4)
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/Tree.tsx (1)
1454-1454
: 上下文传递实现恰当,建议补充文档
disabledStrategy
通过 Context 传递给子组件的实现方式正确。但建议:
- 在组件文档中详细说明该策略函数的使用方法
- 提供示例代码展示不同场景下的使用方式
- 说明与原有 disabled 属性的优先级关系
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/Tree.tsx
(3 hunks)src/TreeNode.tsx
(1 hunks)src/contextTypes.ts
(1 hunks)src/interface.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/interface.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/TreeNode.tsx
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/contextTypes.ts (1)
73-73
: 验证相关组件的实现
需要确保 Tree 组件和 TreeNode 组件正确使用了这个新增的策略函数。
✅ Verification successful
Let me gather more context about the implementation to ensure proper verification.
disabledStrategy 已正确实现并集成
根据验证结果,disabledStrategy 的实现已经完整且正确:
- TreeNode 组件通过 context 正确接收并使用了 disabledStrategy
- 在 isDisabled 方法中正确应用了该策略函数
- Tree 组件已经将该属性正确传递给子组件
实现符合预期,无需进一步修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 Tree 组件和 TreeNode 组件中 disabledStrategy 的使用情况
echo "检查 Tree 组件中的实现:"
ast-grep --pattern 'disabledStrategy' src/Tree.tsx
echo "检查 TreeNode 组件中的实现:"
ast-grep --pattern 'disabledStrategy' src/TreeNode.tsx
Length of output: 305
Script:
#!/bin/bash
# 检查 Tree 和 TreeNode 组件中与 disabled 相关的完整实现上下文
echo "Tree 组件中的 disabled 相关实现:"
ast-grep --pattern 'disabled' src/Tree.tsx
echo -e "\nTreeNode 组件中的 disabled 相关实现:"
rg -A 5 -B 5 'disabledStrategy' src/TreeNode.tsx
echo -e "\n检查是否有其他组件使用 disabledStrategy:"
rg 'disabledStrategy' --type ts --type tsx
Length of output: 754
Script:
#!/bin/bash
# 检查完整的实现上下文
echo "检查 TreeNode 的 context 定义:"
rg -A 10 -B 10 'context:.*{' src/TreeNode.tsx
echo -e "\n检查 disabledStrategy 在所有文件中的使用:"
rg 'disabledStrategy' .
echo -e "\n检查 Tree 组件中的 context 提供:"
ast-grep --pattern 'Provider' src/Tree.tsx
Length of output: 810
src/Tree.tsx (2)
183-183
: 类型定义正确且清晰
disabledStrategy
属性的类型定义采用了可选的函数类型,参数和返回值类型定义明确。
1406-1406
: 属性解构符合现有模式
disabledStrategy
的解构方式与组件中其他属性保持一致。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #890 +/- ##
=======================================
Coverage 99.84% 99.84%
=======================================
Files 14 14
Lines 1321 1323 +2
Branches 384 394 +10
=======================================
+ Hits 1319 1321 +2
Misses 2 2 ☔ View full report in Codecov by Sentry. |
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
🧹 Outside diff range and nitpick comments (1)
src/TreeNode.tsx (1)
234-239
: 代码逻辑正确,但可以优化实现建议通过以下方式优化代码:
- 使用可选链操作符来简化策略调用
- 确保
data
属性的类型安全性- return !!(treeDisabled || disabled || (disabledStrategy && disabledStrategy(data))); + return !!(treeDisabled || disabled || disabledStrategy?.(data));🧰 Tools
🪛 Biome (1.9.4)
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/Tree.tsx
(3 hunks)src/TreeNode.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Tree.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/TreeNode.tsx
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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: 2
🧹 Outside diff range and nitpick comments (2)
src/TreeNode.tsx (2)
235-241
: 建议优化 isDisabled 方法的实现当前实现可以通过以下方式改进:
- 使用可选链操作符
- 使用提前返回模式提高可读性
建议按照以下方式重构:
isDisabled = () => { const { disabled, data } = this.props; const { context: { disabled: treeDisabled }, internalContext: { nodeDisabled }, } = this.props; - return !!(treeDisabled || disabled || (nodeDisabled && nodeDisabled(data))); + if (treeDisabled || disabled) { + return true; + } + + return !!nodeDisabled?.call(null, data); };🧰 Tools
🪛 Biome (1.9.4)
[error] 241-241: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
601-607
: 建议使用 useContext Hook 重构消费者组件当前实现使用嵌套的 Context.Consumer 组件,建议使用 React 的 useContext Hook 来提高代码可读性。
建议按照以下方式重构:
- const ContextTreeNode: React.FC<TreeNodeProps> = props => ( - <TreeContext.Consumer> - {context => ( - <InternalContext.Consumer> - {internalContext => ( - <InternalTreeNode {...props} context={context} internalContext={internalContext} /> - )} - </InternalContext.Consumer> - )} - </TreeContext.Consumer> - ); + const ContextTreeNode: React.FC<TreeNodeProps> = props => { + const context = React.useContext(TreeContext); + const internalContext = React.useContext(InternalContext); + + return ( + <InternalTreeNode + {...props} + context={context} + internalContext={internalContext} + /> + ); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/TreeNode.tsx
(4 hunks)src/contextTypes.ts
(2 hunks)src/index.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/contextTypes.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/TreeNode.tsx
[error] 241-241: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
src/TreeNode.tsx (2)
5-5
: 导入语句正确添加!
新增的 InternalContext
导入与相关上下文导入正确分组。
20-20
: 接口定义正确!
internalContext
属性使用 React.ContextType<typeof InternalContext>
类型定义合理。
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
🧹 Outside diff range and nitpick comments (3)
tests/Tree.spec.tsx (3)
1059-1061
: 代码格式需要优化这里的空行使用不一致,建议保持统一的代码格式。
- treeRef.current.scrollTo({ key: 'light', align: 'top' }); - }); - + treeRef.current.scrollTo({ key: 'light', align: 'top' }); + });
1096-1096
: 代码格式需要优化这里的空行使用不一致,建议保持统一的代码格式。
- const treeRef = React.createRef<any>(); - + const treeRef = React.createRef<any>();Also applies to: 1105-1105
1241-1241
: 多余的空行建议删除这个多余的空行以保持代码整洁。
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/Tree.spec.tsx
(5 hunks)
🔇 Additional comments (1)
tests/Tree.spec.tsx (1)
1242-1332
: 新增的 nodeDisabled 测试用例覆盖全面
测试用例完整地覆盖了以下场景:
- 通过 nodeDisabled 函数禁用节点
- 与可选择树的交互
- 与可勾选树的交互
- 覆盖 disabled 属性的优先级
测试代码结构清晰,断言合理。
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
🧹 Outside diff range and nitpick comments (2)
src/TreeNode.tsx (1)
235-241
: 建议使用可选链访问 nodeDisabled 函数当前实现可以通过使用可选链来优化对 nodeDisabled 函数的访问,使代码更加健壮。
建议修改为:
- return !!(treeDisabled || disabled || (nodeDisabled && nodeDisabled(data))); + return !!(treeDisabled || disabled || nodeDisabled?.(data));🧰 Tools
🪛 Biome (1.9.4)
[error] 241-241: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
tests/Tree.spec.tsx (1)
1059-1061
: 建议增加更多的滚动测试用例当前的滚动测试覆盖了基本场景,建议添加以下测试用例以提高覆盖率:
- 无效 key 的处理
- 边界值的处理
- 不同 align 选项的处理
Also applies to: 1096-1105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/TreeNode.tsx
(4 hunks)src/contextTypes.ts
(1 hunks)src/index.ts
(1 hunks)tests/Tree.spec.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.ts
- src/contextTypes.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/TreeNode.tsx
[error] 241-241: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/TreeNode.tsx (2)
5-5
: 导入和接口更新看起来不错!
新增的 UnstableContext 导入和 unstableContext 属性定义清晰明确。
Also applies to: 20-20
601-607
: Context 组合实现得很好!
通过嵌套 Consumer 来传递多个 context 的方式非常合理,保持了良好的代码组织。
tests/Tree.spec.tsx (1)
1242-1332
: 测试用例编写得很完善!
nodeDisabled 功能的测试覆盖了以下关键场景:
- 基本的节点禁用功能
- 与可选择树的交互
- 与可勾选树的交互
- disabled 属性的覆盖行为
测试结构清晰,断言合理。
Summary by CodeRabbit
UnstableContext
,增强了树节点的禁用状态管理。InternalTreeNode
和ContextTreeNode
以支持新的上下文属性。KeyEntities
类型的格式,使其更易读。