TreeGrid scrollToIndex problem on large tree with expanded nodes

[tarekoraby]

Description

TreeGrid scrollToIndex is not working properly on a large tree with expanded nodes.

Steps to reproduce

In a 14.4.10 project, add the following to a view and click on the button

TreeGrid<String> grid = new TreeGrid<>();
grid.addHierarchyColumn(String::toString).setHeader("Name");
TreeData<String> td = grid.getTreeData();
td.addItem(null, "root");
for (int i = 0; i < 100; i++) {
    td.addItem("root", "child " + i);
    for (int k = 0; k < 100; k++) {
        td.addItem("child " + i, "grand-child " + i + "-" + k);
    }
}
Button button = new Button("Expand and scroll to index 500", e -> {
    grid.expandRecursively(Stream.of("root"), 2);
    grid.scrollToIndex(500);
    // UI.getCurrent().beforeClientResponse(grid, ctx -> grid.scrollToIndex(500));
});
add(grid, button);

Expected outcome

The tree expands and the scrolls to index 500

Actual outcome

The tree expands, but no scrolling occurs.

A second button click is required to scroll to the index. Notice, also that workaround of calling UI.getCurrent().beforeClientResponse(grid, ctx -> grid.scrollToIndex(500)); (which works in the case of shallow trees) doesn't seem to work with large trees with expanded nodes.

[sissbruecker]

After investigating this we concluded that the current API of scrollToIndex(index) can not work in a lazy-loaded, multi-level hierarchy, if index refers to the effective index of an item in the flattened hierarchy. The tree grid simply does not have the information to identify:

• which item is referred to by the index
• what the path through the tree is to arrive at that item
• and thus which nodes / levels need to be fetched from the data provider to be able to display that item

In order for the original use-case to work we would need to introduce a new feature and new API. There are several existing ideas that could be explored:

• Introduce a new API that allows to specify the node path through the tree to arrive at a specific item internal Slack link
• Introduce eager loading to the tree grid, so that children of an expanded node are always loaded eagerly Github link. That would allow gradually expanding nodes until the specific item is loaded. At that point the grid could identify the scroll position for that item.

We also need to deal with the existing scrollToIndex(index) API. Some ideas here:

• deprecate / remove the API
• repurpose the API so that the index only refers to root level nodes. So scrolling to index 500 would then not take all expanded nodes into account, but would try to scroll to node 500 of the root level.

[rolfsmeds]

Of the options for the existing scrollToIndex API, I think the preferrable option would be to redefine it to work only with root nodes, as that is still a useful feature in cases where scrolling to nested nodes isn't necessary. This would need to be reflected in javadoc and documentation.

A new API/feature would need a separate enhancement ticket, and presumably some more investigation to assess the feasibility and solution details.

[Chris-SP365]

To be able to scroll to a specific index, even if it's lazy-loaded multi-level we flatten out and determine the index on the server side. This is some sort of workaround. The new propsed API here would be much better. But it somehow works (given that we also expand the items to the level, and try to scroll per js afterwards again which is not very smart).

Since redefining the purpose of the method would break existing code I tend to vote for deprecation/removal.

Also I have problems understanding why the tree does not know how to scroll to the index: The tree knows its expanded nodes and if a user manually scrolls to a specific position the tree must also show the correct subview. Since scrollPx / cellHeight = index (in a simple tree with identical cell heights) that looks very similar to the scrollToIndex method.

But I'm of course ok with the proposal. Just my 2 cents.

[sissbruecker]

@INPUTsys-Chris

Also I have problems understanding why the tree does not know how to scroll to the index: The tree knows its expanded nodes and if a user manually scrolls to a specific position the tree must also show the correct subview.

On the client-side the tree grid uses lazy-loading which means it does not know about the whole structure of the tree, it knows only about the parts that were scrolled into view at some point. So while it knows which nodes are expanded it does not know how many child nodes an expanded node has unless the loading for that sub-tree happens. As such it can not calculate an effective scroll index since it doesn't know how many nodes in the flattened hierarchy need to be skipped, or even how long the effective hierarchy is.

Consider the following (simplified) example. In the example the grid can only show 5 items at once, which means it will only load the first five root items of the tree. Then we issue a command to expand node 7 - that has no effect so far, as node 7 is not loaded yet. Now we want to scroll to node 7-5 which should have the effective index 12. However the grid does not know yet that node 7 has 5 children, which would affect the effective length of the hierarchy. Instead it will skip right over node 7 and its children and scroll to node 12.

Visible Area (Loaded):
1
2
3
4
5
Invisible Area (Unloaded):
6
7
- 7-1
- 7-2
- 7-3
- 7-4
- 7-5 -> effective index 12 for whole hierarchy
8
9
10
11
12 -> effective index 12 for the known hierarchy when starting scrolling
13
14
15

Maybe the confusion also comes from comparing this to manual scrolling. When scrolling manually the scroll position is changed incrementally, which gives the grid time to catch up with loading further parts of the tree. When scrolling programmatically that happens instantaneously, there is no opportunity for the grid to load the missing nodes in between.

Anyway, hope that clears it up a bit 🙂

[Chris-SP365]

Thanks for your detailed description. I understand the explanation but the scrollbar of the tree indicates the tree does know the size including expanded nodes. If your explanation would be true, the scrollbar thumb would be changing height by scrolling. I've never seen that. And this is also the reason I beleive it should work: The destination scroll pos is reachable. The only problem we face is that it takes time to expand the nodes and that's a timing issue IMHO.

I'm talking about Java API only, so maybe the pure JS tree grid may have these problems.

[sissbruecker]

If your explanation would be true, the scrollbar thumb would be changing height by scrolling. I've never seen that

Maybe you haven't had any obvious examples, but it will definitely do that. The scrollbar will only update when expanding nodes, if those nodes are part of the visible area (note the grid fetches some additional nodes around the visible area to prevent empty content when scrolling).
Otherwise the scrollbar will only update after scrolling to and fetching these expanded nodes. That behaviour is in place so that an operation like "expand the whole tree" does not fetch the whole tree structure.

[Rob-Rocket]

@INPUTsys-Chris yes, it is more a problem on the JS side, because of timing (waiting for expanded nodes to be rendered within the DOM - otherwise the correct node is not reachable)

I did a workaround by determining all subnodes within the path from a root node to the node to scroll to. And then scroll to the root/subnodes recursivly (and expand them), an event is fired (on the JS side, if the tree-side changes - done by a mutation observer (height changed) - then do the ScrollTo(index).

This approach basicaly works in many cases. But it fails, if the Cache is not filled (which will work, if I scroll manually - the cache will be filled and scrollTo(index) will succed. I don't know how the cache is filled in the treeGrid, but I think, this is not done correctly, because of the lazy loading mechanism.

Unfortunatly there is no parameter for the (tree)grid to set the amount of nodes to be rendered.
Maybe this could be good improvement to manipulate the behavior of the lazy loading mechanism.

[Chris-SP365]

That behaviour is in place so that an operation like "expand the whole tree" does not fetch the whole tree structure.

@sissbruecker
Yes I know that from the grid but i didn't know it exists in tree, too. Tested that and you're right.
Thank you for that clarification.

[Chris-SP365]

@Rob-Rocket we did similar and it works for us, too. But of course a new stable API without workarounds would be great.

Good to have your workaround (which is better than ours) documented here in case someone else has problems until the new API is here.

Thanks

[sosa-vaadin]

Due to the difficulties encountered when using the scrollToIndex(int) method with TreeGrid, we're discouraging its use with TreeGrid. The official documentation for TreeGrid and the JavaDoc for the method were updated to warn users about these issues.

A new issue was opened to eventually provide an implementation with predictable behavior.

[TatuLund]

Just pointing earlier comment #1016 (comment)

Introduce eager loading to the tree grid, so that children of an expanded node are always loaded eagerly Github link. That would allow gradually expanding nodes until the specific item is loaded. At that point the grid could identify the scroll position for that item.

This has been effectively done now: #2648