angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.38k stars 6.75k forks source link

Mat-tree :Tree with nested nodes adding child doesnt update the view. #11381

Open juggernut opened 6 years ago

juggernut commented 6 years ago

Bug:

When using mat-nested-tree-node with a nested datasource if you add a child to an existing parent it does not update the view.

What is the expected behavior?

Well if you change or add a child to an existing parent in the nestedDataSource it should update the tree.

What is the current behavior?

It does not update the view when adding a child to an existing parent to the nestedDataStructure .

What is the use-case or motivation for changing an existing behavior?

Well in my case i need to update the view since i add/delete nodes .

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

versions 6 (Chrome ver 66)

Is there anything else we should know?

I found an workaround: refreshTree(){ let _data = this.nestedDataSource.data; this.nestedDataSource.data = null; this.nestedDataSource.data = _data; }

josephperrott commented 6 years ago

Can you please provide a reproduction in stackblitz

wojiusihshen1 commented 6 years ago

I have met the same issue. Could anyone please help confirm whether its a bug?

this.dataChange.next(data);

When update with new object ( e.g data = [] or a new instance), it works. When push/update 'data' object, tree view will not be re render, though this.nestedDataSource.data already updated.

juggernut commented 6 years ago

I think is a bug since it works in one case but not in the other one ! Feel free to use my workaround , meanwhile 👍 .

refreshTree(){ let _data = this.nestedDataSource.data; this.nestedDataSource.data = null; this.nestedDataSource.data = _data; }

tech-no-logical commented 6 years ago

I think the cdk-tree (the basis of mat-tree) has the same issue, see here:

https://stackblitz.com/edit/cdk-nested-tree?file=src%2Fapp%2Fmytree%2Fmytree.component.ts

clicking the 'add' button should push a new node onto the children array. console shows it does, but the tree is not updated.

dodgeviper commented 6 years ago

it also does not update the tree node, if we update the node. https://stackblitz.com/angular/onaqbrpjaan saving a new label to the node does not update the tree.

quanterion commented 6 years ago

Have the same issue. Any updates?

jamadou commented 6 years ago

I've had the same issue. Appears that binding and UI reflecting works only for the first layer of the tree nodes, but not for any nodes down further. Glad to see there is a workaround, thanks to @juggernut

nvuhung commented 5 years ago

Any updates for this? I must use refresh method from @juggernut to refresh the datasource

zhaodong-wang commented 5 years ago

@juggernut's fix seems only work for the data source but treeControl still fails, the toggle doesn't work after updating the data.

SirZach commented 5 years ago

Closed because it was fixed? If we have to use the workaround it should be documented before the issue is closed, right?

juggernut commented 5 years ago

Sorry i am just stupid ... closed by mistake :)

sadplace commented 5 years ago

Any update on this? or estimation when should be fixed?

juggernut commented 5 years ago

Is a P4 problem (considered low priority ) i dont have the time to see if i can do something in the source code . If i have the time i will try to make a pull request my self . But dont hold your breath ( i am really busy ) . Hopefully someone better than me gets to this problem before me !

bijuML commented 5 years ago

@juggernut's fix seems only work for the data source but treeControl still fails, the toggle doesn't work after updating the data.

Yeah I reloaded the datasource but after I am unable to get the indexOf a node using below treeControl.dataNodes.indexOf( node ) Above returns -1

palpatine1991 commented 5 years ago

This problem should be considered higher priority. The workaround introduces quite a serious performance issues for big trees. But it is currently the only way how to implement the asynchronous loading of the nodes

vpatov commented 5 years ago

Any updates on this? I am about to implement a dynamically loaded folder menu and I wanted to use the nested data source/ nested tree control, but judging by this thread it seems I'm better off using the flat data source?

Priemar commented 5 years ago

@vpatov sadly there is currently no progress at it seems. Did you try using trackBy from mat-tree ? I didn't test it but if its working for child items, it will improve the performance when resetting the whole tree source...

But thats not the solution i know, and it depends on your usecase.

Update:

I tested the trackBy and it works for all items in the tree. This mean you can use this as workaround combined with resetting the source (as meantioned already).

In case the tracked => same items will not be rerendered, 1) the performance is better 2) expand / collapsed nodes will keep their state.

Update 2:

Sorry i was to fast with my comment, and i tested one of my old custom implemenations of mat-tree, which handles the trackBy stuff a bit different. Its a long time ago i implemented that tree but i thought i used the default behavior from mat-tree, which was not the case....

While creating a quick example in stackblitz (with the default mat-tree) i realized no matter if you use trackBy nodes will be rerenderd.

but trackBy combined with the "source to null workaround" is NOT improving any rendering performance. As in the mat-tree docu trackBy only helps to improve performance for treeOperations....

Anyway: here is the example (thats exactly the workaround the TO wrote, nothing new and not clean..):

https://stackblitz.com/edit/angular-kc77e4

sorry for spreading hope

juggernut commented 5 years ago

@Priemar add some code example , it helps people :) .

vpatov commented 5 years ago

@Priemar Thank you for your efforts, I am just using the workaround for now because it works fine for me, and doesn't seem to have a strong performance impact.

Rlcolli4 commented 5 years ago

Any update on this? We are needing this behavior as well.

MightGod commented 4 years ago

For me, it DOES updates the view (I used the example "Tree with checkboxes" from here), but when adding a node to a LEAF node, it doesn't refreshing the tree well, as it doesn't present the EXPAND button for the leaf node (which is no leaf at all after adding a node under it).

The WORKAROUND for me was to save aside root's node when initializing the tree, and then in every node addition, call this method:

private refreshTree(): void {
    this.treeControl.collapse(this.rootTree);
    this.treeControl.expand(this.rootTree);
}

This causing the root tree to get collapsed and get expanded again, which builds the view properly after the changes.

jakobadam commented 4 years ago

The cdk tree only renders updates of the backing datastore when the IterableDiffer sees a difference, https://github.com/angular/components/blob/master/src/cdk/tree/tree.ts#L215.

Therefore tree updates are not triggered when objects references stays the same. In order to solve that properly, all objects from the root to the relevant updated node must be updated, aka an immutable update https://redux.js.org/recipes/structuring-reducers/immutable-update-patterns/

Would be nice if the documentation mentioned that, and in addition, had an example of an update.

Here's an example of removing / adding nodes to a nested data source. It replaces relevant nodes on the path from the root to the changed node. And it keeps the expansion state.

https://angular-material-nested-tree-with-updates.stackblitz.io/

Br. Jakob

pmalde commented 4 years ago

Hi Jakob,

Thanks for the info. Could you please share the stack blitz that contains the code for you example?

Kind Regards

adrianriepl commented 4 years ago

Could you please share the stack blitz that contains the code for you example?

Here it is: https://stackblitz.com/edit/angular-material-nested-tree-with-updates

gusarov commented 4 years ago

@adrianriepl @jakobadam

Having Ngrx & immutable js in reducer - job is perfectly done, and additional state cloning here is kind of defeating the purpose of immutable js, I think? Also I have 'normalized' model to help minimize cloning of a graph. The list of children is just an ids, and there is a Map of nodes in a state to navigate by id. I've been so excided by resultant performance of this approach, till I noticed that nested tree is barely works.

Is there a way to actually change IterableDiffer, or add some external hints, or manually trigger renderNodeChanges? The thing is - I'm aware about every change because reducer just did the job. I think I can even add the hint into state itself or add a side effect (ngrx effect) with hints, but I don't want to waste resources and let IterableDiffer or sometihng to actually find what was changed in the tree, this sounds like extra dummy work if reducer have this knowledge. May be any ideas?

UPD: I just found some inspiration here!

janpauldahlke commented 3 years ago

2021 still open, since so i bump, cuz iam having the described problem. will check the "workarounds"

mlota commented 3 years ago

I've got the same issue too. Using MatTreeFlatDataSource I'm able to add new nodes to existing parents but when adding to a leaf node the changes are not being detected. I haven't had much luck with the workarounds either. Any other ideas?

SadiinsoSnowfall commented 3 years ago

Same problem, adding/removing a root node is fine most of the time, but changing a child node does not trigger an update. The workaround does work but for large tree is can take several hundreds of milliseconds.

I found another workaround which does not involve changing the datasource reference: after changing a child node, change the corresponding root node in-place with a shallow copy of itself.

// one of the child nodes of nestedDataSource.data[5] was changed
const tmp = this.nestedDataSource.data;
tmp[5] = Object.assign({}, tmp[5]);
this.nestedDataSource.data = tmp;

The tmp variable is important because we need to trigger the nestedDataSource.data setter to update the tree.

lujian98 commented 3 years ago

Spent lots of time try to figure out to solve this issue. Above workaround works in some way, but when try to drag tree node over another node to expand that node with add children to that node, above workaround will not work.

At the end found this https://stackoverflow.com/questions/53919593/angular-material-tree-does-not-show-child-elements-with-array-is-populated and demo: https://stackblitz.com/edit/angular-addchildtonestedtree will solve my add/remove node children. Still need special take care the root node, possible if initial node children is null when add node children.

RodrigoAN97 commented 2 years ago

Trying to revive this in 2022, any solutions apart from the workaround?

CharlieGreenman commented 2 years ago

It probably makes sense for the community to have a custom pipe setup for tree-cdk and mat-tree. Something I am going to think on. Not being able to implement using your class ngrx/store | async scenario is a but cumbersome. If I do this I will let people now and make it public

Cubelaster commented 2 years ago

I solved this for my needs. I have quite a complex scenario so it will take some time to get it set up on StackBlitz The main point after which it started working is setting up a correct trackBy. It DOES NOT work without trackBy. Also, important thing to note is immutability. Since you're dealing with arrays (most probably you at least have children) and Angular uses shallow compare (or something similar) it won't detect changes until you change the reference of a children ARRAY. Example is this:

new = (parent?: TreeNode<T>): void => {
        const newItem = this.defaultItemGenerator(parent);

        const newNode = {
            children: [],
            isLeaf: true,
            isVisible: true,
            level: parent?.level ?? 0,
            parent,
            value: newItem
        } as TreeNode<T>;

        if (parent) {
            const realParent = this.flatNodeMap.get(parent);

            realParent.children
                ? (realParent.children = [...realParent.children, newNode])
                : (realParent.children = [newNode]);
            realParent.isLeaf = false; // Important as it subsequently decides which template to use in html

            this.dataSource.data = [...this.dataSource.data];

            this.treeControl.expand(parent);
        } else {
            this.dataSource.data = [...this.dataSource.data, newNode];
        }
    };

Be warned: This will definitely be very slow at some point. Right now it takes around 100ms for delete/add for me but in my case it's acceptable. I probably could make it faster but oh well, that's for future me.

Cubelaster commented 2 years ago

Even though it's not fully complete, you can check this StackBlitz. https://stackblitz.com/edit/angular-ivy-9zah3t

I will complete it at some point but the main part is done. Main parts are treeList.ts and treeList.html. Those are the ones showing what you need to do. Original code was somewhere on the StackOverflow and it had basic checkboxes implemented. I expanded on it quite a lot and ended up with a component able to render checkboxes and dynamic tree list, depending on the Inputs passed to it. Notice it's using FlatTreeControl and not NestedTreeControl and that it's API can return an Array and a TreeList at the same time. Anyways, the most important part is trackBy. It will NOT work without it. There are some comments in code if you get lost. You're essentially looking for new and delete methods.

StefaniToto commented 2 years ago

Just made some update below:

This doesnt work for my case let _data = this.nestedDataSource.data; this.nestedDataSource.data = null; this.nestedDataSource.data = _data;

  1. Changing the data doesnt update the template: (checkbox xhanges - see the example)
  2. The only solution for moment to reload the datasource.data that will cause the tree to be collapsed all. So somehow need to track it and save the expanded nodes manually. To be toggled the child node and parent by recursive function.
  3. Im adding new data parent and nodes on every expand (thats my requirements)

https://stackblitz.com/edit/angular-table-tree-example-ybjqmy?file=app%2Ftable-basic-example.ts&file=app%2Ftable-basic-example.html

Cubelaster commented 2 years ago

@StefaniToto I had the same issue and tried to handle it in same way, until I found that trackBy handles it for me. That's the sole purpose of trackBy.

Check the stackblitz I added up there. Setting dataSource to null also did not work for me, but setting it to new reference correctly triggered rerender of the tree. Also, when I added trackBy, the tree was able to track what to rerender (instead of everything every time). Additionally, think about collapsing and expanding nodes, instead of recreating them.

CharlieGreenman commented 1 year ago

Just to keep this thread going. TrackbyFn and pipe are the right approaches. It should be noted, that based on the current architecture of trackBy within angular, data is limited. I.e. you only have the current index, and the current item. Ideally, especially within a flat data structure, there should be the sibling before, and the sibling after within the trackby function. This way, when an item is removed, it can trigger the sibling before and the sibling thereafter. This would then propagate up or down the tree until no more items are affected.

Having to re-render the entire tree, especially for larger folder/file trees will cause serious performance issues. I am working on a trackBy pipe that takes siblings into account and will pass it along here.

However, I think that this also signals that the trackBy function is lacking critical data that would help it's trackBy functionality as well. I.e. sibling before and sibling after. something like this: index, item, itemBefore, itemAfter

CharlieGreenman commented 1 year ago

I should add, i was eventually able to solve my issue via modifying how state management and my services approached changing data. So, I'm not sure if others like me here are complaining due to the actual component being broken or their services/algorithms aren't working as expected. It just might be that trees have the most complex data of ant other angular material component.

dennis-f commented 1 year ago

I don't know what I am doing wrong but i cannot set dataSource.data as it seems a private.

Also the trackby will prevent displaying changes in the node when - like in my example - i add node.bookmark = true, it will not reflect these changes in ui.

I did solve the issues by doing everything recursive, which I originally did not want to. So when changing any data, i will recursively do shallow copies of every node and it's children. I think that is very poor, performance wise, but our trees are quite small, but at least all changes are displayed correctly. Unfortunately I now have to save and reset the expand state for all nodes as well 🙄.

  private deepCopyChildren(node: TreeNode): TreeNode {
    node = {...node};

    if ('children' in node && node.children.length) {
      node.children = node.children.map(subNode => this.deepCopyChildren(subNode));
    }

    return node;
  }

I find that kind of quirky, but wanted to share that anyway. Maybe it helps someone else 👍.

rafakwolf commented 1 year ago

it's 2023, still facing the same issue :D

StefaniToto commented 1 year ago

Sorry for material but had to turn to more advanced lib. Recommended below. http://swimlane.github.io/ngx-datatable/#fullscreen-tree

Cubelaster commented 1 year ago

The component will never get updated at this point. The issues have been reported for years and devs show no intention of changing anything. The component itself is not broken though. It's just really difficult to setup correctly. TrackBy should be a function that would get a new value on each change meant to trigger rerender of a node. Something like a hash or something. Anyways, keep in mind that Angular Change Detection can't properly handle objects (it does reference compare which is why either nulling the node and recreating, or doing a spread will fix this issue) and that TrackBy should be the one to do it instead. Additionally, Angular is funny in a way that sometimes you need to trigger rerender manually, in this case forcing redraw of node by either doing collapse and then open or something else.

ruveey commented 1 year ago

2023, the problem's still present. I hope they will fix it someday.

dennis-f commented 1 year ago

2023, the problem's still present. I hope they will fix it someday.

have you tried the recursive update function?! https://github.com/angular/components/issues/11381#issuecomment-1349055351

I think this is actually the only solution that will work, because it will trigger changedetection.

jakobadam commented 1 year ago

Worked OK for me, when using immutable updates, back in 2020.

Jump up to old comment

neodescis commented 1 year ago

I just tried using MatTree in Angular 16, and it basically doesn't work at all. Performance is terrible, it doesn't save expand/collapse state when using immutable updates, and change detection is spotty at best. Thankfully, there's also not much of a reason to use it. A simple recursive template structure rendered with ng-container is superior in almost every way, except that you have to have something to manage the expanded/collapsed state, e.g. a BehaviorSubject.

tuxflo commented 12 months ago

While the recursive workaround might work somehow it is still kind of ugly, requires somewhat complicated code and one is still not able to use that easily when the tree structure is received from an API. I think what people would like be able to do is something like:

[dataSource]="dataSource$ | async"

(with automatic updates if a nested child was updated) Also the provided 'immutable update' solution is for nested trees, a proper solution should work for flat trees as well.

joelcoxokc commented 10 months ago

Yo 2024 now.. This is a 6 yeah bug...

joelcoxokc commented 10 months ago

Guys i'll be honest. After going through the code. You are going to be way better off implementing your own tree component. This does not seems as performant as simply rendering nested components using @for()

I literally just made a ui-tree and ui-tree-node

the ui-tree-node is used internally in the ui-tree.

And all you do is pass a ng-template into the content of the ui-tree to handle the custom nodes.

Then do some dead simply logic.

Angular's amazing binding and change detection will handle the rest

I personally believe this mat-tree was overly configured

zarend commented 10 months ago

Hello folks,

I would like to remind everyone that Angular has a Code of Conduct.

Anyone gonna fix something as stupid simple as this?

Let's try to keep the tone professional, so that everyone can feel comfortable participating in this open source project.

Best Regards,

Zach

CharlieGreenman commented 10 months ago

Hey this started popping up in my notifications. Passing along a public gist of pipe that I created to get around this one: https://gist.github.com/CharlieGreenman/5352cec2424f9f20d11d48a5de27d8ac

Can then do something like

<-- HTML file --> 
<cdk-tree
      [dataSource]="tree"
      [treeControl]="treeControl"
      [trackBy]="( [ 'id', 'level', 'name'] | trackByProperty )"
// ts file 
treeControl = new FlatTreeControl<any>(
    (node) => node.level,
    (node) => node.expandable
  );

This will help the performance aspect and can then make the cdk-tree immutable without suffering performance issues. Happy coding