Skip to content
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

fix(cdk/tree): react properly to expansion changes #29751

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

BobobUnicorn
Copy link
Collaborator

Fixes #29669. This converts parts of the rendering pipeline to use signals which allows Angular to properly detect changes to the underlying data which may be caused by rendering new nodes. rxjs didn't handle this correctly as the data subscriptions were only set after the initial callback was run, causing partial updates.

@BobobUnicorn BobobUnicorn requested a review from a team as a code owner September 17, 2024 19:44
@BobobUnicorn BobobUnicorn requested review from crisbeto and wagnermaciel and removed request for a team September 17, 2024 19:44
src/cdk/tree/tree.ts Outdated Show resolved Hide resolved
src/cdk/tree/tree.ts Outdated Show resolved Hide resolved
src/cdk/tree/tree-using-legacy-key-manager.spec.ts Outdated Show resolved Hide resolved
src/cdk/tree/tree.ts Outdated Show resolved Hide resolved
@BobobUnicorn BobobUnicorn force-pushed the fix/cdk-tree-synchronous-expansion branch from c1115f6 to 433e357 Compare September 20, 2024 19:36
}

/** @docs-private */
@Directive({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is private, do we need a separate component for it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure of another way to hook into the onInit for the directive here, though I think ultimately removing mostRecentTreeNode would solve this issue (e.g. doing data passing through injection)

src/cdk/tree/tree.ts Outdated Show resolved Hide resolved
@BobobUnicorn BobobUnicorn force-pushed the fix/cdk-tree-synchronous-expansion branch from b138b69 to cb4edf0 Compare November 18, 2024 20:40
@BobobUnicorn
Copy link
Collaborator Author

fixed the merge conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(tree): Using cdk-tree with ArrayDataSource and synchronous data, the isExpanded functionality can fail.
2 participants