refactor(material/tree): fork tests for using and not using tree control #27925
Conversation
Fork mat-tree tests to mat-tree-using-tree-control.Remove uses of deprecated members from mat-tree tests. Tests in mat-tree-using-tree-control remain unchanged. This commit message will be squashed away after merging in cdk-tree-revamp.
zarend
force-pushed
the
cdk-tree-revamp-backwards-compatible
branch
from
1ac5a5f
to
4c2069a
Compare
Fix backwards compatibility issue for Trees created before CdkTreeNode#isExpandable was added. Steps to reproduce. 1. Create a example tree using MatNestedTreeControl. Be sure to *not* provide an isExpandable function. 2. Add a parent and child node to example. 3. (App renders a tree with two nodes, parent has chrevron icon on it). 4. Navigate to parent node and press right arrow key on keyboard 5. (Expecting node to expand, but nothing seems to happen). Work-around: click on the node instead of using keyboard Change isExpandable logic of CdkTreeNode. Current behavior is that tree nodes are not expandable unless isExpandable is provided. With this commit applied, tree nodes that have a tree control will be expandable by default. Unless isExpandable is provided, trees using childrenAccessor or levelAccessor are not expandable, but trees using TreeControl are expandable.
|
|
||
| // For compatibility with trees created using TreeControl before we added | ||
| // CdkTreeNode#isExpandable. | ||
| return true; |
There was a problem hiding this comment.
it's not clear to me if backwards compatibility is desired in this case given that the original behaviour is incorrect from an a11y standpoint
There was a problem hiding this comment.
Angular components is required to follow semantic versioning. When releasing this in a minor version, we're required not to break apps that were made before before introducing CdkTreeNode#isExpandable.
I know that the a11y is incorrect, but this is not causing an a11y regression. It means that consumers will get the correct behavior when moving off the the deprecated Tree Control. I know this isn't particularly satisfying, but we have to follow Angular's semantic versioning policy.
If it were for a major version, then we would have a bit more flexibility, but doing this in a minor will allow us to release this in a timely manor. There are a lot of a11y improvements in this PR, and I think it would be good to do what we can feasibly land.
I'm definitely open to suggestions if there is another way we can do this that is backwards compatible for Trees made before cdk-tree-revamp.
There was a problem hiding this comment.
We should document this then, I can make some edits to the readmes to that effect. (i.e. upgrade to use this API to get a11y improvements)
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fork mat-tree tests to mat-tree-using-tree-control.Remove uses of deprecated members from mat-tree tests. Tests in mat-tree-using-tree-control remain unchanged.
This commit message will be squashed away after merging in cdk-tree-revamp.