treeGrid.getDataProvider().refreshAll() annoyingly scrolls to the start. See video.

[enver-haase]

Description of the bug

When I re-validate the tree, I expect this to be a no-op if there is nothing to do. Certainly it should not scroll. This is bad UX.

Expected behavior

Re-validating a validated tree should be a no-op. In any case, when I drop an item using drag-and-drop, the tree should not jump away from underneath the mouse pointer.

Minimal reproducible example

package org.vaadin.example;

import com.vaadin.flow.component.Unit;
import com.vaadin.flow.component.button.Button;
import com.vaadin.flow.component.grid.Grid;
import com.vaadin.flow.component.grid.dnd.GridDropLocation;
import com.vaadin.flow.component.grid.dnd.GridDropMode;
import com.vaadin.flow.component.notification.Notification;
import com.vaadin.flow.component.textfield.TextArea;
import com.vaadin.flow.component.treegrid.TreeGrid;

import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.data.provider.hierarchy.TreeData;
import com.vaadin.flow.router.Route;

import java.util.ArrayList;
import java.util.List;


@Route
public class MainView extends VerticalLayout {

    private static class DemoTreeData extends TreeData<String> {
        private DemoTreeData() {
            final String[] five = {"A", "B", "C", "D", "E"};

            addRootItems(five);

            for (String s : five) {
                List<String> children = new ArrayList<>();
                for (String ch : five) {
                    String child = s+ch;
                    children.add(child);
                }

                addItems(s, children);
            }
        }
    }
    public MainView() {

        TextArea textArea = new TextArea("History");
        textArea.setReadOnly(true);
        textArea.setSizeFull();

        TreeGrid<String> treeGrid = new TreeGrid<>();

        treeGrid.addHierarchyColumn(String::toString).setHeader("Name");

        treeGrid.setTreeData(new DemoTreeData());

        treeGrid.setDropMode(GridDropMode.ON_TOP_OR_BETWEEN);
        treeGrid.setSelectionMode(Grid.SelectionMode.SINGLE);
        treeGrid.setRowsDraggable(true);

        List<String> dragged = new ArrayList<>();
        treeGrid.addDragStartListener( evt -> {
            dragged.clear();
            dragged.addAll(evt.getDraggedItems());
        } );

        treeGrid.addDropListener( evt -> {
            try {
                if (GridDropLocation.ON_TOP == evt.getDropLocation()) {
                    evt.getDropTargetItem().ifPresent(dropTarget -> {
                        for (String drag : dragged) {
                            if (drag.equals(dropTarget)) {
                                throw new IllegalStateException("Ignored dropping to itself.");
                            }
                            if (isAncestor(treeGrid.getTreeData(), dropTarget, drag)) {
                                throw new IllegalStateException("Cannot make an ancestor a child of its descendant.");
                            }
                            treeGrid.getTreeData().setParent(drag, dropTarget);
                            treeGrid.getDataProvider().refreshAll();
                            textArea.setValue(textArea.getValue()+drag+" -> "+dropTarget+"\n");
                        }
                    });
                }
            }
            catch (Exception e) {
                Notification.show("Exception: "+e);
            }
        } );


        Button button = new Button("Revalidate!", e -> treeGrid.getDataProvider().refreshAll());


        treeGrid.setWidthFull();
        treeGrid.setHeight(500, Unit.PIXELS);
        add(treeGrid, textArea, button);
    }

    /**
     * Tests whether a potential ancestor (a parent or grandparent or grandgrand...parent) of a given node.
     * @param treeData The tree data structure
     * @param node the node
     * @param potentialAncestor the potential ancestor
     * @return true if and only of potentialAncestor is a genealogical predecessor of node
     * */
    private static <T> boolean isAncestor(TreeData<T> treeData, T node, T potentialAncestor) {
        if (potentialAncestor == null) {
            return true; // null is the parent of the 'root nodes'.
        }
        T parent = node;
        while ((parent = treeData.getParent(parent)) != null){
            if (potentialAncestor.equals(parent)) {
                return true;
            }
        }
        return false;
    }

}

Versions

• Vaadin / Flow version: 24.3.11
• Java version: n/a
• OS version: n/a
• Browser version (if applicable): n/a
• Application Server (if applicable): n/a
• IDE (if applicable): n/a

[enver-haase]

https://github.com/vaadin/flow/assets/7870436/4e18e1e8-a464-4044-87e8-08576b79b730

[TatuLund]

Looks like duplicate of: https://github.com/vaadin/flow-components/issues/1236

[TatuLund]

With the following change issue goes away

Replace Button code with:

        Button button = new Button("Revalidate!",
                e -> refreshChildItems(treeGrid, treeData.getRootItems()));

And add recursive refreshItem method

    private void refreshChildItems(TreeGrid<String> grid,
            List<String> rootItems) {
        for (String item : rootItems) {
            grid.getDataProvider().refreshItem(item, false);
            refreshChildItems(grid, grid.getTreeData().getChildren(item));
        }
    }

See also: https://github.com/vaadin/flow-components/issues/1050#issuecomment-957465512

[rolfsmeds]

Javadocs for TreeGrid refreshAll should mention the limitation and propose using refreshItem instead.

[enver-haase]

If I move one root item under another one, so that

 A
 B

becomes

> B (with A underneath)

then I cannot refresh the underlying DataProvider in such a way that A disappears from the view, using only refreshItem().

[stefanuebe]

Minor side note to the workaround of Tatu. It works indeed, but it is also significant slower. We have a case, where the refresh time goes from ~5s to around 35s (a tree grid with roughly 44k rows and all expanded initially).

So not suitable for all situations.

[randomuser1231232]

Accoring to this comment https://github.com/vaadin/flow/issues/21071#issuecomment-2754063124 refreshItem(item, true) does not support hierarchy(adds/deletes/moves) changes and when using it, you will run into many other issues.

Please add a deepRefresh functionality that works on any item and actually refreshes the whole subtree (refresh item itself, refresh all descendants, add added descendants, remove deleted descendants, remove decendants of collapsed descendants).

This would fix so many issues developers currently have with TreeGrid.

[randomuser1231232]

To actually workaround this issue, is it possible to get scrollTop of the grid before calling refreshAll and then restoring scrollTop after refreshAll finishes?

[vursen]

Hey! We are considering introducing a new data provider that is intended to solve this problem. You can find more details in vaadin/flow-components#7269. Feedback is very welcome!

[vursen]

Resolved by https://github.com/vaadin/platform/issues/7843 in Vaadin 25.

You can now use the new HierarchyFormat.FLATTENED feature to prevent scroll position from being affected on refreshAll. For more details and to share feedback, see: https://vaadin.com/forum/t/feedback-needed-treegrid-flat-hierarchy-support-in-vaadin-25/178634