vaadin / flow-components

Java counterpart of Vaadin Web Components
82 stars 63 forks source link

TreeGrid does not or not reliably scroll to an item, or expand its nodes from the root to an item. See video. #6312

Open enver-haase opened 1 month ago

enver-haase commented 1 month ago

Description of the bug

I should be able to scroll any item into view, and also expand the parent nodes. But even the calculation of the parent node(s) does not work reliably.

Expected behavior

I'd like to have a simple 'expandRouteToItemAndScrollIntoView(T item)'. I'd like the existing API to function as expected.

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.orderedlayout.VerticalLayout;
import com.vaadin.flow.component.treegrid.TreeGrid;
import com.vaadin.flow.data.provider.hierarchy.HierarchicalDataCommunicator;
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 ScrollTreeGrid<T> extends TreeGrid<T> {
        /**
         * @param item the item where to scroll to.
         */
        public void scrollToItemAndExpand(T item) {

            HierarchicalDataCommunicator<T> dataCommunicator = super.getDataCommunicator();
            ArrayList<T> items = new ArrayList<>();
            items.add(item);

            T parent;
            do {
                parent = dataCommunicator.getParentItem(items.getLast());
                Notification.show("Item '" + item + "' has parent '" + parent + "'.");
                if (parent != null) {
                    items.add(parent);
                }
            }
            while (parent != null);
            // items now has all the items from 'item' up to a root item (which it could be itself).

            expand(items.reversed());

            Notification.show("Scrolling to item '" + item + "', the index being '" + dataCommunicator.getIndex(item) + "'.");
            scrollToIndex(dataCommunicator.getIndex(item));
        }
    }

    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();

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

        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("Scroll to DC!", e -> treeGrid.scrollToItemAndExpand("DC"));

        treeGrid.setWidthFull();
        treeGrid.setHeight(200, 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

enver-haase commented 1 month ago

https://github.com/vaadin/flow/assets/7870436/82bdaaf6-d997-48b1-b320-285d5b6b6b70

TatuLund commented 1 month ago

I think you use wrong scrollToIndex method in the code, see: https://vaadin.com/docs/next/components/tree-grid#programmatic-scrolling

enver-haase commented 1 month ago

That has to do with the fact that I cannot see any method that would return such an array of int, in order to parametrize scrollToIndex(int...).

rolfsmeds commented 1 month ago

Unfortunately you need to figure out the indexes yourself based on the data you feed into the TreeGrid. Ideally, you would instead use scrollToItem, but that doesn't work in TreeGrid. These are known issues for which we have not been able to find a solution yet.

enver-haase commented 1 month ago

In many cases, a scrollToSelected() with no parameters would be good enough. Has that been considered?

enver-haase commented 1 month ago

In my experiments, I could see that dataCommunicator.getParentItem(T item) is unreliable (works only when the item's parent is expanded). That's not documented and needs to be repaired.

In case the TreeGrid has a TreeData, that data structure's getParent can be used instead. It can also be used to find the int[] needed for scrollToIndex(int...) like so:

parent = getTreeData().getParent(current); // this can be considered back-end access
int index = getTreeData().getChildren(parent).indexOf(current);
indices.add(index);

in a loop from item to root, then reverse the indices list and do

int[] in = indices.reversed().stream().mapToInt(Integer::intValue).toArray();
scrollToIndex(in);

in the same loop, the parental chain can also be recorded and later be used for node-by-node expansion.