CirclonGroup / angular-tree-component

A simple yet powerful tree component for Angular (>=2)
https://angular2-tree.readme.io/docs
MIT License
1.1k stars 492 forks source link

fix: do not load whole child collection with virtual scroll (#463) #868

Closed Novkirishki closed 3 years ago

Novkirishki commented 4 years ago

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ X ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #463

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ X ] No

Other information

tobiasengelhardt commented 4 years ago

Hi @Novkirishki, thank you for this PR! I tested it so far and it looks good. But I have some trouble reproducing the issue. You said in the issue that it happens when the nodeHeight tree option is INodeHeightFn and not a number. What function are you using and what are you returning?

Novkirishki commented 4 years ago

Hi @tobiasengelhardt, thank you for the quick response! Here is how I reproduce the issue: commit.

The INodeHeightFn function I use simply returns a number. I have added a dummy component to easily track how many instances are created. I have also added a getChildren() function so that the child nodes are loaded async.

When I expand the node with children for the first time I can see in the console that 1000 instances of the dummy component are created. If I collapse the node, further expansions work as expected and only create around 25-6 dummy components, because children height is already calculated and cached. With the fix from the PR there are only 25-6 nodes created on the initial expansion as well.

tobiasengelhardt commented 3 years ago

Very sorry for the late merge. This fix is now included in version 10.0.1. Thank you again for the contribution!