-
Notifications
You must be signed in to change notification settings - Fork 403
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
feat: support treeview treeItem.checkboxstate #4214
Conversation
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
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. Walkthrough此拉取请求引入了多个与树视图节点复选框状态管理相关的更改。主要包括在 Changes
Possibly related PRs
Suggested reviewers
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 (8)
packages/extension/src/browser/vscode/api/tree-view/tree-view.node.defined.ts (2)
61-61
: 建议为新增的复选框相关属性添加文档注释为了提高代码的可维护性,建议为
_checkboxInfo
参数和checkboxInfo
getter 添加 JSDoc 文档注释,说明其用途和类型。+ /** 树节点的复选框状态信息 */ private _checkboxInfo?: TreeViewItemCheckboxInfo, + /** 获取树节点的复选框状态信息 */ get checkboxInfo() { return this._checkboxInfo; }Also applies to: 96-98
155-155
: 建议提取复选框相关逻辑到基类或接口中
ExtensionCompositeTreeNode
和ExtensionTreeNode
类中的复选框相关代码存在重复。建议考虑以下重构方案:
- 创建一个包含复选框功能的基类
- 或创建一个复选框接口并使用组合模式
这样可以减少代码重复,提高可维护性。
Also applies to: 189-191
packages/extension/src/browser/components/extension-tree-view.tsx (2)
120-128
: 建议增强复选框状态变更处理的健壮性当前实现可以考虑添加以下改进:
- 添加错误处理
- 添加日志记录以便调试
- 考虑添加防抖动处理,避免快速重复点击
const handleCheckBoxChange = useCallback( (item: ExtensionTreeNode | ExtensionCompositeTreeNode) => { + try { const { handleCheckBoxChange } = model; if (item) { + console.debug('[TreeView] Checkbox state changed for item:', item.treeItemId); handleCheckBoxChange(item); } + } catch (error) { + console.error('[TreeView] Failed to handle checkbox change:', error); + } }, - [model], + [model] );
240-240
: 建议为接口属性添加文档注释为了提高代码的可读性和可维护性,建议为
TreeViewProps
接口中的handleCheckBoxChange
属性添加详细的文档注释。+ /** + * 处理树节点复选框状态变更的回调函数 + * @param item - 发生变更的树节点 + */ handleCheckBoxChange(item: ExtensionTreeNode | ExtensionCompositeTreeNode): void;packages/extension/src/browser/components/extension-tree-view-node.tsx (2)
197-213
: 建议增强复选框的可访问性当前复选框实现可以通过以下方式提升可访问性:
- 添加 aria-label 属性
- 确保键盘可访问性
- 添加高对比度支持
return ( <CheckBox data-node-id={node.treeItemId} readOnly onChange={(event) => handleCheckBoxChange(event)} checked={!!node.checkboxInfo.checked} id={node.treeItemId} + aria-label={node.checkboxInfo.accessibilityInformation?.label || `${node.displayName} checkbox`} + tabIndex={0} role={node.checkboxInfo.accessibilityInformation?.role} title={node.checkboxInfo.tooltip} + className={styles.highContrastCheckbox} /> );
160-166
: 建议增强复选框事件处理当前的事件处理可以通过以下方式改进:
- 添加键盘事件支持(空格键)
- 防止在加载状态时触发
- 添加状态变更的过渡动画
const handleCheckBoxChange = useCallback( (event: FormEvent<HTMLInputElement>) => { + if (loading) { + return; + } onChange(item); event.stopPropagation(); + // 添加过渡动画类 + event.currentTarget.classList.add(styles.checkboxTransition); }, - [item, onChange], + [item, onChange, loading], ); + const handleKeyDown = useCallback( + (event: KeyboardEvent<HTMLInputElement>) => { + if (event.key === ' ') { + event.preventDefault(); + handleCheckBoxChange(event as unknown as FormEvent<HTMLInputElement>); + } + }, + [handleCheckBoxChange], + );packages/extension/src/browser/vscode/api/tree-view/tree-view.model.service.ts (1)
1087-1095
: 建议添加空值处理和方法文档建议对该方法做以下改进:
- 添加对
checkboxInfo
为 undefined 的处理- 添加方法文档说明参数和功能
+/** + * 处理树节点复选框状态变更 + * @param item 要更新状态的树节点 + */ handleCheckBoxChange = async (item: ExtensionCompositeTreeNode | ExtensionTreeNode) => { - if (item) { + if (item && item.checkboxInfo) { this.treeViewDataProvider.markAsChecked( item, !item.checkboxInfo!.checked, this.treeViewOptions.manageCheckboxStateManually, ); } };packages/components/src/recycle-tree/tree/TreeNode.ts (1)
247-253
: 建议优化性能和类型安全性当前实现在每次访问时都创建新对象,建议进行以下优化:
- 缓存返回值以提高性能
- 使用只读类型增加类型安全性
- get checkboxInfo(): TreeViewItemCheckboxInfo | undefined { + private _checkboxInfo?: Readonly<TreeViewItemCheckboxInfo>; + + get checkboxInfo(): Readonly<TreeViewItemCheckboxInfo> | undefined { + if (!this._checkboxInfo) { + this._checkboxInfo = Object.freeze({ + checked: false, + tooltip: '', + accessibilityInformation: this.accessibilityInformation, + }); + } + return this._checkboxInfo; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
packages/components/src/recycle-tree/tree/TreeNode.ts
(2 hunks)packages/components/src/recycle-tree/types/tree-node.ts
(2 hunks)packages/extension/src/browser/components/extension-tree-view-node.tsx
(6 hunks)packages/extension/src/browser/components/extension-tree-view.tsx
(5 hunks)packages/extension/src/browser/vscode/api/main.thread.treeview.ts
(4 hunks)packages/extension/src/browser/vscode/api/tree-view/tree-view.model.service.ts
(1 hunks)packages/extension/src/browser/vscode/api/tree-view/tree-view.node.defined.ts
(5 hunks)packages/extension/src/common/vscode/treeview.ts
(5 hunks)packages/extension/src/hosted/api/vscode/ext.host.treeview.ts
(7 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/browser/vscode/api/main.thread.treeview.ts
[error] 625-626: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (17)
packages/extension/src/common/vscode/treeview.ts (6)
42-42
: 添加新方法 $checkStateChanged
新增方法 $checkStateChanged
可以正确地同步复选框状态,与现有接口保持一致。
81-85
: 添加 TreeViewItemCheckboxInfo 接口
TreeViewItemCheckboxInfo
接口的添加,为树视图项提供了复选框状态信息,设计合理。
87-91
: 添加枚举 TreeItemCheckboxState
新增枚举 TreeItemCheckboxState
,定义了复选框的状态,符合功能需求。
113-114
: 在 TreeViewItem 类中添加 checkboxInfo 属性
为 TreeViewItem
类增加可选属性 checkboxInfo
,增强了树视图项的功能扩展,结构清晰。
133-138
: 添加事件 onDidChangeCheckboxState
为 TreeView
接口添加事件 onDidChangeCheckboxState
,以响应复选框状态的变化,设计合理。
197-201
: 在 TreeViewBaseOptions 中添加 manageCheckboxStateManually 属性
新增可选属性 manageCheckboxStateManually
,允许手动管理复选框状态,提供了更大的灵活性。
packages/extension/src/browser/vscode/api/main.thread.treeview.ts (3)
2-2
: 添加必要的导入声明
新增导入 CompositeTreeNode, ITreeNodeOrCompositeTreeNode, Tree
,补充了所需的类型定义。
417-417
: 在 createFoldNode 方法中添加 checkboxInfo 参数
为 createFoldNode
方法添加 item.checkboxInfo
参数,正确地将复选框信息传递给节点,逻辑清晰。
441-441
: 在 createNormalNode 方法中添加 checkboxInfo 参数
为 createNormalNode
方法添加 item.checkboxInfo
参数,确保普通节点也能包含复选框信息。
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts (5)
227-233
: 添加方法 $checkStateChanged
在 ExtHostTreeViews
类中新增方法 $checkStateChanged
,用于更新复选框状态,实现了与前端的同步。
326-328
: 添加事件 onDidChangeCheckboxState
在 ExtHostTreeView
类中添加 onDidChangeCheckboxState
事件,用于监听复选框状态的变化。
371-371
: 传递 manageCheckboxStateManually 选项
在注册树数据提供者时,正确地将 manageCheckboxStateManually
选项传递给代理,确保配置生效。
656-671
: 实现 checkStateChanged 方法
checkStateChanged
方法正确地更新了元素的复选框状态,并触发相应事件,逻辑清晰。
801-816
: 在 toTreeViewItem 方法中处理 checkboxInfo
正确地将 treeItem.checkboxState
转换为 checkboxInfo
,确保复选框信息在树视图项中传递。
packages/components/src/recycle-tree/types/tree-node.ts (2)
6-11
: 添加 TreeViewItemCheckboxInfo 接口
新增 TreeViewItemCheckboxInfo
接口,为树节点添加复选框状态信息,设计合理。
50-53
: 在 ITreeNode 接口中添加 checkboxInfo 属性
为 ITreeNode
接口添加 checkboxInfo
可选属性,增强了节点的功能扩展。
packages/extension/src/browser/components/extension-tree-view-node.tsx (1)
2-12
: 导入声明的组织结构合理
React 相关导入和组件库导入已经按照类型分组,结构清晰。
Also applies to: 14-21
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4214 +/- ##
==========================================
- Coverage 54.02% 53.99% -0.03%
==========================================
Files 1616 1616
Lines 98044 98100 +56
Branches 20063 20074 +11
==========================================
+ Hits 52967 52973 +6
- Misses 37451 37488 +37
- Partials 7626 7639 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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 (3)
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts (3)
223-235
: 代码实现正确,建议添加错误处理方法实现逻辑正确,但建议增加对 items 参数的空值检查。
$checkStateChanged(treeViewId: string, items: { treeItemId: string; checked: boolean }[]): Promise<void> { + if (!items?.length) { + return Promise.resolve(); + } const treeView = this.treeViews.get(treeViewId); if (!treeView) { throw new Error('No tree view with id ' + treeViewId); } return treeView.checkStateChanged(items); }
658-673
: 建议优化状态转换逻辑当前实现正确但可以更简洁。建议使用 map 替代 forEach,并添加类型注解提高代码可读性。
async checkStateChanged(items: readonly { treeItemId: string; checked: boolean }[]): Promise<void> { - const transformed: [T, TreeItemCheckboxState][] = []; - items.forEach((item) => { + const transformed: [T, TreeItemCheckboxState][] = items.map((item) => { const node = this.getTreeItem(item.treeItemId); - if (node) { - transformed.push([node, item.checked ? TreeItemCheckboxState.Checked : TreeItemCheckboxState.Unchecked]); - const treeViewItem = this.element2TreeViewItem.get(node); - if (treeViewItem && treeViewItem.checkboxInfo) { - treeViewItem.checkboxInfo.checked = item.checked; - } + if (!node) { + return null; } - }); + const treeViewItem = this.element2TreeViewItem.get(node); + if (treeViewItem?.checkboxInfo) { + treeViewItem.checkboxInfo.checked = item.checked; + } + return [node, item.checked ? TreeItemCheckboxState.Checked : TreeItemCheckboxState.Unchecked]; + }).filter((item): item is [T, TreeItemCheckboxState] => item !== null); this.onDidChangeCheckboxStateEmitter.fire({ items: transformed, }); }
803-818
: 复选框状态转换逻辑完善状态转换逻辑考虑了所有可能的情况,包括未定义、对象形式和简单布尔值。建议添加注释说明不同状态的处理逻辑。
+// 处理复选框状态的三种情况: +// 1. undefined - 不显示复选框 +// 2. object - 包含状态、提示和辅助信息 +// 3. boolean - 仅包含状态 let checkboxInfo; if (isUndefined(treeItem.checkboxState)) { checkboxInfo = undefined; } else if (isObject(treeItem.checkboxState)) { checkboxInfo = { checked: treeItem.checkboxState.state === TreeItemCheckboxState.Checked, tooltip: treeItem.checkboxState.tooltip, accessibilityInformation: treeItem.checkboxState.accessibilityInformation, }; } else { checkboxInfo = { checked: treeItem.checkboxState === TreeItemCheckboxState.Checked, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts
(8 hunks)packages/types/vscode/typings/vscode.d.ts
(1 hunks)
🔇 Additional comments (3)
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts (2)
328-329
: 事件发射器命名清晰且符合约定
事件发射器的命名和实现符合代码库的一般模式。
373-373
: 配置选项集成良好
manageCheckboxStateManually 选项的集成位置正确,保持了与其他选项的一致性。
packages/types/vscode/typings/vscode.d.ts (1)
Line range hint 2159-2171
: 代码变更看起来不错!
这个新增的 checkboxState
属性设计得很好,特别是:
- 使用了 readonly 来确保状态的不可变性
- 包含了无障碍功能支持
- 提供了自定义 tooltip 的能力
建议验证一下这个新属性与 TreeView 事件系统的集成情况,特别是与 onDidChangeCheckboxState
事件的交互。
✅ Verification successful
根据搜索结果,我可以生成最终的验证结果。代码显示 TreeView 接口已经完整实现了复选框状态变更的事件系统:
复选框功能集成验证完成
TreeView 接口已经正确实现了复选框状态变更的支持:
- 在 TreeView 接口中定义了
onDidChangeCheckboxState
事件 - 完整支持了
TreeCheckboxChangeEvent<T>
事件类型 - 提供了
TreeItemCheckboxState
枚举来表示复选框状态
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 TreeView 相关事件处理
ast-grep --pattern $'interface TreeView {
$$$
onDidChangeCheckboxState: Event<$_>;
$$$
}'
Length of output: 96
Script:
#!/bin/bash
# 搜索 TreeView 接口定义和相关事件
rg "interface TreeView" -A 20
# 搜索 TreeItemCheckboxState 的定义和使用
rg "TreeItemCheckboxState" -A 5
# 搜索与 checkbox 状态变化相关的事件定义
rg "onDidChangeCheckboxState|TreeCheckboxChangeEvent" -A 5
Length of output: 35354
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts
(8 hunks)
🔇 Additional comments (6)
packages/extension/src/hosted/api/vscode/ext.host.treeview.ts (6)
11-13
: 导入必要的类型判断函数
代码中正确地导入了 isNumber
、isString
和 isUndefined
,用于类型检查,符合需求。
26-26
: 正确地引入了 TreeItemCheckboxState
成功引入了 TreeItemCheckboxState
,为后续处理树状视图中的复选框状态提供了支持。
229-235
: 实现了 $checkStateChanged
方法
新增的 $checkStateChanged
方法正确地处理了复选框状态的更新,检查了 treeView
是否存在,并调用了相应的 checkStateChanged
。实现逻辑清晰,符合预期。
658-673
: 正确实现了 checkStateChanged
方法
checkStateChanged
方法正确地更新了复选框状态,并触发了 onDidChangeCheckboxStateEmitter
事件,逻辑清晰。
830-830
: 在 treeViewItem
中正确包含了 checkboxInfo
成功地将 checkboxInfo
添加到 treeViewItem
中,确保复选框状态信息能被正确传递和使用。
805-817
: 🛠️ Refactor suggestion
调整条件判断以确保类型正确性
为了确保准确判断 treeItem.checkboxState
的类型,建议调整条件判断逻辑。当前的逻辑可能在类型判断上存在问题,建议修改为:
let checkboxInfo;
- if (isUndefined(treeItem.checkboxState)) {
- checkboxInfo = undefined;
- } else if (!isNumber(treeItem.checkboxState)) {
+ if (isNumber(treeItem.checkboxState)) {
+ checkboxInfo = {
+ checked: treeItem.checkboxState === TreeItemCheckboxState.Checked,
+ };
+ } else if (!isUndefined(treeItem.checkboxState)) {
checkboxInfo = {
checked: treeItem.checkboxState.state === TreeItemCheckboxState.Checked,
tooltip: treeItem.checkboxState.tooltip,
accessibilityInformation: treeItem.checkboxState.accessibilityInformation,
};
+ } else {
+ checkboxInfo = undefined;
}
这样调整后,代码将首先检查 checkboxState
是否为数字类型,然后再处理对象类型,逻辑更加清晰,类型判断也更加准确。
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.
LGTM
Types
Background or solution
Support treeview treeItem.checkboxstate
Changelog
Summary by CodeRabbit
新功能
TreeViewItemCheckboxInfo
,提供复选框相关信息。文档