refactor!: expand on scroll to index and throw if not found (#8109)

ugur-vaadin · vursen · web-flow · commit c22b9530c49b · 2025-10-03T17:10:23.000+03:00
* feat: add scrolltoitem for treedataproviders * test: remove expand param * test: refactor test page to separate test data * test: move static fields * test: expose test data methods * Revert "refactor: extract logic that will reside in data providers" This reverts commit 57483aa. * refactor: extract data provider logic into methods with same signatures * refactor: cleanup and simplify * Revert "refactor: cleanup and simplify" This reverts commit 3f40845. * test: add unit tests for scrolltoitem * chore: remove demo page * Update vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/TreeGridScrollTest.java Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * chore: run formatter * refactor: use data provider api and update docs * test: add new providers * chore: remove todo * test: update unit tests for custom data providers * test: fix typo in test name * refactor: move logic to data communicator do not determine item for nested * test: add tests for items that do not belong to the tree * refactor: get data provider once * refactor: extract communicator and expand on scroll to item * chore: fix compilation * refactor: separate resolve item method * Revert "refactor: separate resolve item method" This reverts commit 3425eab. * refactor: expand after index path is resolved * fix: expand before getting index path * refactor: throw exception in communicator and return int array * test: remove lazy provider tests and cleanup * Update vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/ScrollToItemTest.java Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/ScrollToItemTest.java Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/ScrollToItemTest.java Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * test: cleanup * docs: update table with links * docs: remove link * refactor: expand on scroll to index and throw if not found * chore: run formatter * test: fix it assertion * chore: run formatter * refactor: throw exception in communicator * refactor: use extracted client side scroll method * refactor: remove unnecessary expand * fix: add missing item check for scroll to single index * refactor: do not throw on item not found and support negative indexes --------- Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridScrollToIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridScrollToIT.java
@@ -114,7 +114,7 @@ public void expandAll_scrollToItem30_1_correctFirstVisibleItem() {
     @Test
     public void scrollToIndex30_1_correctFirstVisibleItem() {
         scrollToIndexInput.sendKeys("30-1", Keys.TAB);
-        assertFirstVisibleRowContent("Granddad 30");
+        assertFirstVisibleRowContent("Dad 30/1");
     }
 
     @Test
diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/treegrid/TreeGrid.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/treegrid/TreeGrid.java
@@ -956,22 +956,23 @@ public HierarchicalDataProvider<T, SerializablePredicate<T>> getDataProvider() {
      * {@link HierarchyFormat#FLATTENED}: the index refers to an item in the
      * entire flattened tree, not only the root level, allowing items at any
      * expanded level to be reached with this method.
-     * <p>
-     * If the index exceeds the valid range, scrolling stops at the last
-     * available item.
      *
      * @param index
      *            zero based index of the item to scroll to
      */
     @Override
     public void scrollToIndex(int index) {
-        getUI().ifPresent(
-                ui -> ui.beforeClientResponse(this, ctx -> getElement()
-                        .executeJs("this.scrollToIndex($0);", index)));
+        var itemCount = getDataCommunicator().getItemCount();
+        if (index >= itemCount || index < -itemCount) {
+            // The index does not correspond to an item
+            return;
+        }
+        doScrollToIndex(index);
     }
 
     /**
-     * Scrolls to a nested item specified by its hierarchical path.
+     * Scrolls to a nested item specified by its hierarchical path. Any
+     * collapsed parent of the item is expanded before scrolling.
      * <p>
      * The hierarchical path is an array of zero-based indexes, where each index
      * refers to a child of the item at the previous index. Scrolling continues
@@ -1004,24 +1005,27 @@ public void scrollToIndex(int... path) {
                             For HierarchyFormat.FLATTENED, use scrollToIndex(int) with a flat index instead.
                             """);
         }
-
         if (path.length == 0) {
             throw new IllegalArgumentException(
                     "At least one index should be provided.");
         }
-
-        String joinedIndexes = Arrays.stream(path).mapToObj(String::valueOf)
-                .collect(Collectors.joining(","));
-        getUI().ifPresent(ui -> ui.beforeClientResponse(this,
-                ctx -> getElement().executeJs(
-                        "this.scrollToIndex(" + joinedIndexes + ");")));
+        try {
+            var pathItems = ((TreeGridDataCommunicator<T>) getDataCommunicator())
+                    .getPathItems(path);
+            var ancestors = pathItems.subList(0, pathItems.size() - 1);
+            expand(ancestors);
+        } catch (IllegalArgumentException e) {
+            // The path does not correspond to an item
+            return;
+        }
+        doScrollToIndex(path);
     }
 
     @Override
     public void scrollToEnd() {
-        getUI().ifPresent(ui -> ui.beforeClientResponse(this,
-                ctx -> getElement().executeJs(
-                        "this.scrollToIndex(...Array(10).fill(-1))")));
+        var indexes = new int[10];
+        Arrays.fill(indexes, -1);
+        doScrollToIndex(indexes);
     }
 
     /**
@@ -1157,4 +1161,12 @@ public void scrollToItem(T item) {
             scrollToIndex(indexPath);
         }
     }
+
+    private void doScrollToIndex(int... path) {
+        var joinedIndexes = Arrays.stream(path).mapToObj(String::valueOf)
+                .collect(Collectors.joining(","));
+        getUI().ifPresent(ui -> ui.beforeClientResponse(this,
+                ctx -> getElement().executeJs(
+                        "this.scrollToIndex(" + joinedIndexes + ");")));
+    }
 }
diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/treegrid/TreeGridDataCommunicator.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/treegrid/TreeGridDataCommunicator.java
@@ -97,6 +97,8 @@ public int[] resolveItem(T item) {
      * @param ancestors
      *            the ordered list of the ancestors of the item
      * @return index path for the given item
+     * @throws IllegalArgumentException
+     *             if the item does not exist
      */
     private int[] getIndexPath(T item, List<T> ancestors) {
         var path = new ArrayList<Integer>();
@@ -132,6 +134,38 @@ private List<T> getAncestors(T item) {
         return ancestors;
     }
 
+    /**
+     * Gets the ordered list of items for the specified path. Returns empty if
+     * any of the items are not found.
+     *
+     * @param path
+     *            the path to get the items for
+     * @return ordered list of items for the specified path
+     * @throws IllegalArgumentException
+     *             if the path does not correspond to an item
+     */
+    public List<T> getPathItems(int... path) {
+        var dataProvider = (HierarchicalDataProvider<T, Object>) getDataProvider();
+        var pathItems = new ArrayList<T>();
+        T parent = null;
+        for (var index : path) {
+            if (index < 0) {
+                var childrenCount = dataProvider.getChildCount(
+                        buildQuery(parent, 0, Integer.MAX_VALUE));
+                index = childrenCount + index;
+            }
+            var query = buildQuery(parent, index, 1);
+            var childOptional = dataProvider.fetchChildren(query).findFirst();
+            if (childOptional.isEmpty()) {
+                throw new IllegalArgumentException(
+                        "There is no item with the specified path.");
+            }
+            parent = childOptional.get();
+            pathItems.add(parent);
+        }
+        return pathItems;
+    }
+
     private List<Integer> getAncestorPath(List<T> ancestors) {
         var ancestorPath = new ArrayList<Integer>();
         for (var i = 0; i < ancestors.size(); i++) {
diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/ScrollToIndexTest.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/treegrid/ScrollToIndexTest.java
@@ -15,29 +15,135 @@
  */
 package com.vaadin.flow.component.treegrid;
 
+import java.util.stream.IntStream;
+
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 
+import com.vaadin.flow.component.UI;
 import com.vaadin.flow.data.provider.hierarchy.HierarchicalDataProvider.HierarchyFormat;
 import com.vaadin.flow.data.provider.hierarchy.TreeData;
 import com.vaadin.flow.data.provider.hierarchy.TreeDataProvider;
+import com.vaadin.flow.function.DeploymentConfiguration;
+import com.vaadin.flow.server.VaadinContext;
+import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.VaadinSession;
 
 public class ScrollToIndexTest {
-    private TreeGrid<String> treeGrid = new TreeGrid<>();
-    private TreeData<String> treeData = new TreeData<>();
+    private TreeGrid<String> treeGrid;
+    private TreeData<String> treeData;
+    private UI ui;
+
+    @Before
+    public void init() {
+        ui = new UI();
+        UI.setCurrent(ui);
+        var mockSession = Mockito.mock(VaadinSession.class);
+        var mockService = Mockito.mock(VaadinService.class);
+        var mockContext = Mockito.mock(VaadinContext.class);
+        var mockConfiguration = Mockito.mock(DeploymentConfiguration.class);
+        Mockito.when(mockSession.getService()).thenReturn(mockService);
+        Mockito.when(mockSession.getConfiguration())
+                .thenReturn(mockConfiguration);
+        Mockito.when(mockService.getContext()).thenReturn(mockContext);
+        ui.getInternals().setSession(mockSession);
+        treeGrid = new TreeGrid<>();
+        ui.add(treeGrid);
+        treeData = getTreeData();
+    }
+
+    @After
+    public void tearDown() {
+        UI.setCurrent(null);
+    }
 
     @Test
-    public void nestedHierarchyFormat_scrollToIndexPath_doesNotThrow() {
+    public void flattenedHierarchyFormat_scrollToIndexPath_throwsUnsupportedOperationException() {
+        treeGrid.setDataProvider(
+                new TreeDataProvider<>(treeData, HierarchyFormat.FLATTENED));
+        Assert.assertThrows(UnsupportedOperationException.class,
+                () -> treeGrid.scrollToIndex(0, 0));
+    }
+
+    @Test
+    public void nestedHierarchyFormat_scrollToIndexPathWithCollapsedParent_expandsParent() {
         treeGrid.setDataProvider(
                 new TreeDataProvider<>(treeData, HierarchyFormat.NESTED));
-        treeGrid.scrollToIndex(0, 0);
+        var rootItem = treeData.getRootItems().get(10);
+        var firstChild = treeData.getChildren(rootItem).getFirst();
+        Assert.assertFalse(treeGrid.isExpanded(rootItem));
+        Assert.assertFalse(treeGrid.isExpanded(firstChild));
+        treeGrid.scrollToIndex(10, 0);
+        Assert.assertTrue(treeGrid.isExpanded(rootItem));
+        Assert.assertFalse(treeGrid.isExpanded(firstChild));
     }
 
     @Test
-    public void flattenedHierarchyFormat_scrollToIndexPath_throws() {
+    public void flattenedHierarchyFormat_scrollToIndexNotPresent_notScrolled() {
         treeGrid.setDataProvider(
                 new TreeDataProvider<>(treeData, HierarchyFormat.FLATTENED));
-        Assert.assertThrows(UnsupportedOperationException.class,
-                () -> treeGrid.scrollToIndex(0, 0));
+        assertScrollToIndexCalled(false, 1000);
+        assertScrollToIndexCalled(false, -1000);
+    }
+
+    @Test
+    public void flattenedHierarchyFormat_scrollToIndex_scrolled() {
+        treeGrid.setDataProvider(
+                new TreeDataProvider<>(treeData, HierarchyFormat.FLATTENED));
+        assertScrollToIndexCalled(true, 10);
+        assertScrollToIndexCalled(true, -10);
+    }
+
+    @Test
+    public void nestedHierarchyFormat_scrollToPathNotPresent_notScrolled() {
+        treeGrid.setDataProvider(
+                new TreeDataProvider<>(treeData, HierarchyFormat.NESTED));
+        assertScrollToPathCalled(false, 1000);
+        assertScrollToPathCalled(false, -1000);
+        assertScrollToPathCalled(false, 10, 5, 5);
+        assertScrollToPathCalled(false, -10, -5, -5);
+    }
+
+    @Test
+    public void nestedHierarchyFormat_scrollToPath_scrolled() {
+        treeGrid.setDataProvider(
+                new TreeDataProvider<>(treeData, HierarchyFormat.NESTED));
+        assertScrollToPathCalled(true, 10, 5);
+        assertScrollToPathCalled(true, -10, -5);
+    }
+
+    private void assertScrollToIndexCalled(boolean called, int index) {
+        treeGrid.scrollToIndex(index);
+        Assert.assertEquals(called, isScrollToIndexCalled());
+    }
+
+    private void assertScrollToPathCalled(boolean called, int... path) {
+        treeGrid.scrollToIndex(path);
+        Assert.assertEquals(called, isScrollToIndexCalled());
+    }
+
+    private boolean isScrollToIndexCalled() {
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+        ui.getInternals().getStateTree().collectChanges(ignore -> {
+        });
+        var invocations = ui.getInternals().dumpPendingJavaScriptInvocations();
+        return invocations.stream().anyMatch(invocation -> invocation
+                .getInvocation().getExpression().contains("scrollToIndex"));
+    }
+
+    private static TreeData<String> getTreeData() {
+        var rootItemCount = 50;
+        var childItemCountPerRootItem = 10;
+        var treeData = new TreeData<String>();
+        var rootItems = IntStream.range(0, rootItemCount)
+                .mapToObj(i -> "Item " + i).toList();
+        treeData.addRootItems(rootItems);
+        rootItems.forEach(rootItem -> treeData.addItems(rootItem,
+                IntStream.range(0, childItemCountPerRootItem)
+                        .mapToObj(i -> rootItem + "-" + i)));
+        return treeData;
     }
 }
