TreeGrid HierarchicalDataProvider.refreshItem() does not work correctly

[pellmann]

Description

The positions of the nodes in the tree are only refreshed correctly when a HierarchicalDataProvider.refreshAll() is done - but not when a HierarchicalDataProvider.refreshItem() is done.

This becomes visible when a node is created as a child of another and is the last one and there is also more than one.

Expected outcome

In both cases - with refreshAll() and with refreshItem() - the result should be the same and the dragged node should not be null.

Minimal reproducible example

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.grid.dnd.GridDropMode;
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.component.treegrid.TreeGrid;
import com.vaadin.flow.data.provider.hierarchy.AbstractHierarchicalDataProvider;
import com.vaadin.flow.data.provider.hierarchy.HierarchicalDataProvider;
import com.vaadin.flow.data.provider.hierarchy.HierarchicalQuery;
import com.vaadin.flow.function.ValueProvider;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;

public class TestTree extends TreeGrid<TestTree.TestNode> {

    private TestNode sourceNode;

    public TestTree() {
        setSizeFull();

        // simple structure
        TestNode root = new TestNode(null, "root");
        root.addChild(new TestNode(root, "first"));
        TestNode second = root.addChild(new TestNode(root, "second"));
        second.addChild(new TestNode(second, "second.first"));
        second.addChild(new TestNode(second, "second.second"));
        second.addChild(new TestNode(second, "second.third"));
        TestNode third = root.addChild(new TestNode(root, "third"));
        TestNode fourth = third.addChild(new TestNode(third, "fourth"));
        fourth.addChild(new TestNode(fourth, "fourth.first"));
        fourth.addChild(new TestNode(fourth, "fourth.second"));
        fourth.addChild(new TestNode(fourth, "fourth.third"));

        // set data
        setDataProvider(new TestHierarchyProvider(root));

        // set view
        addComponentHierarchyColumn((ValueProvider<TestNode, Component>) TestNode::getView);

        // drag and drop
        setRowsDraggable(true);
        setDropMode(GridDropMode.ON_TOP);
        addDragStartListener(event -> {
            if (!event.getDraggedItems().isEmpty()) {
                sourceNode = event.getDraggedItems().iterator().next();
                if (sourceNode == null) {
                    System.err.println("dragged item is null");
                }
            }
        });
        addDropListener(event -> {
            event.getDropTargetItem().ifPresent(targetNode -> {
                move(sourceNode, targetNode);
            });
        });

        // expand all
        expandRecursively(List.of(root), Integer.MAX_VALUE);
    }

    private void move(TestNode source, TestNode target) {
        TestNode sourceParent = source.getParent();
        target.addChild(source.detach());

        // refresh only the related items do not work
        getDataProvider().refreshItem(sourceParent, true);
        getDataProvider().refreshItem(target, true);

        // refresh all works
//        getDataProvider().refreshAll();
    }

    static public class TestNode {

        private final List<TestNode> children = new ArrayList<>();
        private TestNode parent;
        private String name;

        public TestNode(TestNode parent, String name) {
            this.parent = parent;
            this.name = name;
        }

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public Component getView() {
            return new Span(name);
        }

        public List<TestNode> getChildren() {
            return children;
        }

        public TestNode addChild(TestNode child) {
            children.add(child);
            child.setParent(this);
            return child;
        }

        public TestNode detach() {
            getParent().getChildren().remove(this);
            parent = null;
            return this;
        }

        public TestNode getParent() {
            return parent;
        }

        public void setParent(TestNode parent) {
            this.parent = parent;
        }
    }

    public static class TestHierarchyProvider
            extends AbstractHierarchicalDataProvider<TestNode, String>
            implements HierarchicalDataProvider<TestNode, String> {

        private final TestNode root;

        public TestHierarchyProvider(TestNode root) {
            this.root = root;
        }

        @Override
        public int getChildCount(HierarchicalQuery<TestNode, String> query) {
            TestNode parent = query.getParent();
            if (parent == null) {
                return 1;
            }
            return parent.getChildren().size();
        }

        @Override
        public Stream<TestNode> fetchChildren(HierarchicalQuery<TestNode, String> query) {
            TestNode parent = query.getParent();
            if (parent == null) {
                return Stream.of(root);
            }
            return parent.getChildren().subList(
                    query.getOffset(), Integer.min(query.getOffset() + query.getLimit(),
                            parent.getChildren().size())).stream();
        }

        @Override
        public boolean hasChildren(TestNode item) {
            return !item.getChildren().isEmpty();
        }

        @Override
        public boolean isInMemory() {
            return true;
        }
    }
}

Steps to reproduce

Specifically, the problem arises when one:

• drag fourth.third onto second
• then drag fourth.third (as last child of second) again on fourth
  • PROBLEM: the dragged item is NULL
• this only happens if this is used
  • getDataProvider().refreshItem(sourceParent, true);
  • getDataProvider().refreshItem(target, true);
• this does not happens with
  • getDataProvider().refreshAll();

Environment

Vaadin version(s): 24.1.4 OS: latest macOS

Browsers

Chrome

[pellmann]

When I use refreshAll() (as a workaround), I have the problem that the tree always scrolls completely back to the beginning when it is larger.

My attempt to get back to the position with the variant from the cookbook (https://cookbook.vaadin.com/scroll-to-item-in-tree-grid) unfortunately shows that this also no longer works, currently.

So I am actually somewhat blocked by the behavior.

[tomivirkki]

This issue still seems valid and needs more investigation.

Calling getDataCommunicator().getKeyMapper().remove(source); inside the move method seems to workaround it.