WIP feat(module:tree-view): upgrade tree view #9003
Conversation
|
This preview will be available after the AzureCI is passed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9003 +/- ##
==========================================
+ Coverage 91.94% 92.07% +0.13%
==========================================
Files 559 575 +16
Lines 19773 20264 +491
Branches 3050 3115 +65
==========================================
+ Hits 18180 18659 +479
- Misses 1267 1273 +6
- Partials 326 332 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There has been a lack of testing for |
WwwHhhYran
force-pushed
the
feat/tree_view_accessor
branch
from
bf44c72 to
386deb3
Compare
WwwHhhYran
force-pushed
the
feat/tree_view_accessor
branch
from
386deb3 to
9c549c0
Compare
WwwHhhYran
force-pushed
the
feat/tree_view_accessor
branch
from
9c549c0 to
a18116a
Compare
WwwHhhYran
changed the title
done |
| @Input('nzTreeControl') override treeControl?: TreeControl<T, NzSafeAny> = undefined; | ||
| @Input('nzLevelAccessor') override levelAccessor?: (dataNode: T) => number = undefined; | ||
| @Input('nzChildrenAccessor') override childrenAccessor?: (dataNode: T) => T[] = undefined; | ||
| @Input('nzDataSource') |
There was a problem hiding this comment.
CDKTree 已经重构为 inject 进行依赖注入，其 constructor 为空，这里可以转为 inject 以简化代码
- noAnimation = inject(NzNoAnimationDirective, { host: true, optional: true });
- constructor(
- protected differs: IterableDiffers,
- protected changeDetectorRef: ChangeDetectorRef,
- private directionality: Directionality
- ) {
- super(differs, changeDetectorRef, directionality);
- }
+ protected noAnimation = inject(NzNoAnimationDirective, { host: true, optional: true });
+ protected directionality = inject(Directionality);
+ protected changeDetectorRef = inject(ChangeDetectorRef);| @@ -82,7 +90,8 @@ export class NzTreeNodeComponent<T> extends NzNodeBase<T> implements OnDestroy, | |||
| } | |||
There was a problem hiding this comment.
this extends NzNodeBase extends CdkTreeNode which has empty constructor.
use inject() to simpify
protected get tree(): NzTreeView<T> {
return this._tree as NzTreeView<T>
}- constructor(
- protected elementRef: ElementRef<HTMLElement>,
- protected tree: NzTreeView<T>
- ) {
- super(elementRef, tree);
- this._elementRef.nativeElement.classList.add('ant-tree-treenode');
- }classList.add('ant-tree-treenode') 这个则直接放到 host 里
| @@ -82,7 +90,8 @@ export class NzTreeNodeComponent<T> extends NzNodeBase<T> implements OnDestroy, | |||
| } | |||
|
|
|||
| override ngOnInit(): void { | |||
| this.isLeaf = !this.tree.treeControl?.isExpandable(this.data); | |||
| super.ngOnInit(); | |||
| this.isLeaf = !this.isExpandable; | |||
There was a problem hiding this comment.
这里用 isExpandable 推导 isLeaf 很奇怪，应该是反过来是叶子节点或者传入的 nzExpandable 为 false 来推导 isExpandable。就像 cdk 中这段https://github.com/angular/components/blob/main/src/cdk/tree/tree.ts#L1231-L1241
建议直接弃用 isLeaf 并用 CDKTreeNode 中的 isLeafNode 替代
// node-base.ts
/**
* @deprecated use `isLeafNode` instead
*/
abstract isLeaf: boolean;| @@ -0,0 +1,155 @@ | |||
| --- | |||
There was a problem hiding this comment.
不建议单独开 legacy 的示例文档，此 pr 将进入 v20 版本，大版本更新后的用户直接使用新 API，若要查看旧写法访问历史版本的文档即可
| override set isExpandable(value: boolean) { | ||
| super.isExpandable = value; | ||
| } | ||
|
|
||
| indents: boolean[] = []; | ||
| disabled = false; |
There was a problem hiding this comment.
disabled selected 这几个成员被用来动态地控制宿主元素的 class，在下面的代码中用到了 renderer API 来做这个操作。
这里可以改为 signal 实现来提升性能。
@Component({
//...
host: {
//..
'[class.ant-tree-treenode-disabled]': '_disabled()'
})
// @deprecated
disabled = false;
protected _disabled = signal(false);
disable(): void {
this.disabled = true;
this._disabled.set(true);
}
enable(): void {
this.disabled = false;
this._disabled.set(false);
}旧的 disabled 先标记为 deprecated and will removed ，v21 时移除并将 _disabled 公开成为新的 disabled.
selected 同理
There was a problem hiding this comment.
我有些纠结，我原本是把 tree-view 组件尽可能都 upgrade 到基于 signal 的用法。但后来有两点顾虑：
-
如果其他开发者基于我们的组件进行二次开发，这次升级可能直接导致他们的组件不可用。
-
升级后感觉代码很混乱。我归因于
CdkTree和CdkTreeNode等组件目前几乎没有使用 signal，而我们的组件依赖于它们，某些属性继承与方法重写需要与其保持一致。因此只能在一些其他地方使用 signal，但这会显得代码很杂乱，整体的代码一致性较差，后期维护可能是个问题。
| @@ -72,15 +78,24 @@ export class NzTreeNodeIndentLineDirective<T> implements OnDestroy { | |||
| }); | |||
| } | |||
|
|
|||
| // indents 中的 true 和 false 表示当前节点左侧是否应该有竖线，因为如果不存在 nextSibling，就不存在竖线 | |||
| } | ||
|
|
||
| return null; | ||
| return this.nzCompareBy || (this.treeControl as BaseTreeControl<T, NzSafeAny> | undefined)?.trackBy || null; |
There was a problem hiding this comment.
compareBy 是组件内部的一个用于优化重新渲染的 trick，不建议暴露给用户，和 trackBy 保持一致即可
get compareBy() {
if (this.trackBy) {
return (value: T) => this.innerTrackBy(null, value);
}
return (this.treeControl as BaseTreeControl<T, NzSafeAny> | undefined)?.trackBy || null;
}| this.dataSource.setData(TREE_DATA); | ||
| this.treeControl.expandAll(); | ||
| ngOnInit(): void { | ||
| this.dataSource = new NzTreeViewFlatDataSource(this.tree, this.treeFlattener, TREE_DATA); |
| * It is recommended to use nzChildrenAccessor for now. | ||
| */ | ||
| // this.tree.expandAll(); | ||
| this.tree._getExpansionModel().select(...this.dataSource.getFlattenData()); |
There was a problem hiding this comment.
- 示例仍在 constructor 中 expandAll，最大程度保持与原来用法
- 使用 CDKTree#expandAll 方法
There was a problem hiding this comment.
- tree 是通过
ViewChild拿到的，在constructor中拿不到吧？目前社区主流的方法是在AfterViewInit中调用。
- cdkTree 的
expandAll方法在扁平数据中暂时有问题，主要关于 tree 中的数据和视图管理同步的问题 👇
-
以前使用
TreeControl时，可以在 dataSource 的connect方法中提前根据 tree control 中记录的树展开状态来过滤掉不需要展示的节点，再传递给 tree 进行渲染。- 这样的好处是即使 tree 中只有部分节点，但是 tree control 包含所有节点和节点的状态（expand/collapse)。也就是说视图和数据管理是分开的，所以 expandAll 不会出问题。https://github.com/NG-ZORRO/ng-zorro-antd/blob/7725fbd4fabc5c003badb50cf4c72b3159d721a1/components/tree-view/data-source.ts#L108C3-L120C4
-
现在的问题是移除了数据管理工具
TreeControl，其方法都迁移到了 tree 中。如果我们像以前一样根据节点状态过滤完数据给 tree 的话，这个时候节点数据是不完整的，使用 treeexpandAll方法没办法展开所有数据（collpaseAll同理）。但是如果把所有数据都给 tree 的话，就会始终展示所有节点，此时点击 expand 和 collapse 都没效果。- 官方用了一个比较 hack 的解法：通过 css 实现隐藏不必要节点，参考官方文档。如下：
[style.display] = “shouldRender ? ‘none' ： ’flex‘”ps: 我目前在本地使用了官方文档的写法，但是 css style 的切换无法触发 angular animation, expand 和 collapse 行为目前有些生硬，所以我还没有提交最新的代码上来。
通过阅读 CdkTree 的源码，我怀疑这可能是一个 bug，目前我已经提交了 issue 向 Angular/component 进行确认，之后可能需要根据该 team 的回复做进一步修改。angular/components#30735
| private destroy$ = new Subject<boolean>(); | ||
| dir: Direction = 'ltr'; | ||
| _dataSourceChanged = new Subject<void>(); | ||
| // eslint-disable-next-line @angular-eslint/no-input-rename | ||
| dataNodes: T[] = []; |
There was a problem hiding this comment.
dataNodes 不建议放在 NzTreeView，而是 datasource 里
There was a problem hiding this comment.
- dataSource 的类型不局限于
DataSource结构，也可以是 observable, array，难以统一。 - 放在 dataSource 里不利于在 nzTreeNodeIndentLine 中维护，因为 treeControl、levelAccessor、childrenAccessor 的 dataSource 的数据结构不同（flat / nested）。
- 严格来说 dataSource 是一个可供用户自由设计的对象，存在 dataSource 中还是 tree 实例中都是可以的，我这里是保持 tree control 的历史行为。
Laffery
added
PR: target-major
PR: reviewed-changes-requested
and removed
PR: target-minor
labels
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Upgrade Tree View Component
angular/cdk has now marked
treeControlas deprecated in 18.2.0, replacing it withlevelAccessorandchildrenAccessor, and is planning to removetreeControlin 21.0.0. So I'm going to synchronise this feature in Ng Zorro.So I've made the following changes:
nzTreeControlparameter and addnzLevelAccessorandnzChildrenAccessorparamter, in order to synchronize the new features of CdkTree (@angular/cdk ^18.2.0)levelAccessorandchildrenAccessor.Does this PR introduce a breaking change?
Other information