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.