TreeGrid w/TreeData and Drag & Drop does not internally update. See video.

[enver-haase]

Description of the bug

This is about DX, but I would consider it a bug rather than a feature. TreeData and TreeGrid are Vaadin-provided classes. I feel I should not need to learn about DataCommunicator or that internally the TreeData is wrapped with some HierarchicalDataProvider or such. I should be able to manipulate the TreeData and see the results.

As a bare minimum, this would need to be documented very thoroughly, including how to avoid scrolling or "jumping" when updating.

Expected behavior

Dragging and dropping should not only have an effect in the TreeData, I should immediately see those changes.

Minimal reproducible example

package org.vaadin.example;

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.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() {

    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(s -> {
                    for (String drag : dragged) {
                        treeGrid.getTreeData().removeItem(drag);
                        treeGrid.getTreeData().addItem(s, drag);
                    }
                });
                //treeGrid.getDataCommunicator().reset();
            }
        }
        catch (Exception e) {
            Notification.show("Exception: "+e);
        }
    } );

    treeGrid.setWidthFull();
    treeGrid.setHeight("800px");
    add(treeGrid);
}

}

Versions

• Vaadin / Flow version: 24.3.10
• 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]

Screen.Recording.2024-05-07.at.17.08.20.mov

[mshabarov]

Why do you use DataCommunicator? Isn't call the treeGrid.getDataProvider().refreshAll() be enough, that's a true public API for the component.
Would be good to update the documentation (and javadocs) to mention that the data refresh is needed in this case:
some methods in TreeData, e.g. setParent, have an instruction to call the TreeDataProvider#refreshAll(), but the above methods, used in the example, do not. Thus, we have to update the javadoc for all the method in TreeData at least, then online docs.

[enver-haase]

Why not DataCommunicator?

Or, put differently, where in the documentation does it say
HierarchicalDataProvider<T, SerializablePredicate<T>> getDataProvider()
is the official API? It looks convoluted enough.

As I initially said (and the code has no getDataProvider() and has getDataComminicator() commented out) -- I feel one should not have to learn about the internal workings.
A am setting a TreeData, not a DataProvider of any kind. Getting one DataProvider and notifying it that I changed the TreeData seems like really bad API. In fact, TreeData is a Vaadin class and should already have all the means to update the representation when the model is changed.

If it is true that some methods in TreeData correctly notify the DataProvider that's wrapped around it, then all the data-altering methods should correctly notify the DataProvider about the change.