Improvement to details row height handling. (#11351)

- When rows are added, details row heights need to be taken into account
in positioning.

Fixes #11348

3 files changed, 159 insertions, 2 deletions

diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
index ff76a66f96..cc207c10c0 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -2907,10 +2907,14 @@ public class Escalator extends Widget
              */
             scroller.recalculateScrollbarsForVirtualViewport();
 
+            double spacerHeightsSumUntilIndex = spacerContainer
+                    .getSpacerHeightsSumUntilIndex(index);
             final boolean addedRowsAboveCurrentViewport = index
-                    * getDefaultRowHeight() < getScrollTop();
+                    * getDefaultRowHeight()
+                    + spacerHeightsSumUntilIndex < getScrollTop();
             final boolean addedRowsBelowCurrentViewport = index
-                    * getDefaultRowHeight() > getScrollTop()
+                    * getDefaultRowHeight()
+                    + spacerHeightsSumUntilIndex > getScrollTop()
                             + getHeightOfSection();
 
             if (addedRowsAboveCurrentViewport) {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLast.java b/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLast.java
new file mode 100644
index 0000000000..e7aea5f215
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLast.java
@@ -0,0 +1,62 @@
+package com.vaadin.tests.components.treegrid;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.vaadin.data.TreeData;
+import com.vaadin.data.provider.TreeDataProvider;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TreeGrid;
+
+public class TreeGridDetailsExpandLast extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        TreeData<String> data = new TreeData<>();
+        List<String> roots = new ArrayList<String>();
+        for (int i = 0; i < 100; i++) {
+            String root = "Root " + i;
+            roots.add(root);
+            data.addItem(null, root);
+            for (int j = 0; j < 2; j++) {
+                String leaf = "Leaf " + i + "/" + j;
+                data.addItem(root, leaf);
+            }
+        }
+
+        TreeDataProvider<String> treeDataProvider = new TreeDataProvider<>(
+                data);
+        TreeGrid<String> treeGrid = new TreeGrid<>();
+        treeGrid.setDataProvider(treeDataProvider);
+        treeGrid.addColumn(String::toString).setCaption("String")
+                .setId("string");
+        treeGrid.addColumn((i) -> "--").setCaption("Nothing");
+        treeGrid.setHierarchyColumn("string");
+        treeGrid.setDetailsGenerator(
+                row -> new Label("details for " + row.toString()));
+        treeGrid.addItemClickListener(event -> {
+            treeGrid.setDetailsVisible(event.getItem(),
+                    !treeGrid.isDetailsVisible(event.getItem()));
+        });
+
+        treeGrid.expand(roots);
+        for (String id : roots) {
+            treeGrid.setDetailsVisible(id, true);
+        }
+        treeGrid.collapse("Root 99");
+        addComponent(treeGrid);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Details row heights should be taken into account when expanding rows. "
+                + "Expanding the last row shouldn't cause exceptions or weird row positions.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11348;
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLastTest.java b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLastTest.java
new file mode 100644
index 0000000000..ce6e866ff7
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsExpandLastTest.java
@@ -0,0 +1,91 @@
+package com.vaadin.tests.components.treegrid;
+
+import static org.hamcrest.number.IsCloseTo.closeTo;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.StaleElementReferenceException;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.testbench.elements.TreeGridElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class TreeGridDetailsExpandLastTest extends MultiBrowserTest {
+
+    private TreeGridElement treeGrid;
+
+    private ExpectedCondition<Boolean> expectedConditionDetails(
+            final int root) {
+        return new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver arg0) {
+                return getDetails(root) != null;
+            }
+
+            @Override
+            public String toString() {
+                // waiting for...
+                return String.format("Root %s details row contents to be found",
+                        root);
+            }
+        };
+    }
+
+    private WebElement getDetails(final int root) {
+        String text = "details for Root %s";
+        try {
+            TestBenchElement details = treeGrid.getDetails(root + 2 * root);
+            if (details != null) {
+                // ensure the details belong to the correct row
+                List<WebElement> labels = details
+                        .findElements(By.className("v-label"));
+                for (WebElement label : labels) {
+                    if (String.format(text, root).equals(label.getText())) {
+                        return details;
+                    }
+                }
+            }
+        } catch (StaleElementReferenceException e) {
+            treeGrid = $(TreeGridElement.class).first();
+        }
+        return null;
+    }
+
+    @Test
+    public void expandLastRow() {
+        openTestURL();
+        waitForElementPresent(By.className("v-treegrid"));
+
+        treeGrid = $(TreeGridElement.class).first();
+        waitUntil(expectedConditionDetails(1));
+
+        treeGrid.scrollToRow(297);
+        waitUntil(expectedConditionDetails(99));
+
+        treeGrid.expandWithClick(297);
+
+        assertEquals("Error notification detected.", 0, treeGrid
+                .findElements(By.className("v-Notification-error")).size());
+
+        GridCellElement cell98_1 = treeGrid.getCell(296, 0);
+        GridCellElement cell99 = treeGrid.getCell(297, 0);
+        WebElement spacer99 = getDetails(99);
+
+        assertThat("Unexpected row location.",
+                (double) cell98_1.getLocation().getY()
+                        + cell98_1.getSize().getHeight(),
+                closeTo(cell99.getLocation().getY(), 2d));
+        assertThat("Unexpected spacer location.",
+                (double) cell99.getLocation().getY()
+                        + cell99.getSize().getHeight(),
+                closeTo(spacer99.getLocation().getY(), 2d));
+    }
+}