Fixes to displaying Grid in a detail row. (#11147)

- Multiple headers shouldn't stack behind each other.
- Body rows shouldn't get stuck to default row height.
- Compatibility version's hidable row selector shouldn't try to
calculate row heights based on rows that haven't been added to DOM yet.

Fixes #7674

12 files changed, 337 insertions, 21 deletions

diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java
index 26a2f294b7..7ef6ff6bbf 100644
--- a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java
@@ -236,6 +236,14 @@ public interface RowContainer {
     public int getRowCount();
 
     /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @since
+     * @return {@code true} if row height calculations have been scheduled
+     */
+    public boolean isAutodetectingRowHeightLater();
+
+    /**
      * The default height of the rows in this RowContainer.
      *
      * @param px
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 a3ecfab902..393708f7da 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -1274,6 +1274,8 @@ public class Escalator extends Widget
 
         private boolean initialColumnSizesCalculated = false;
 
+        private boolean autodetectingRowHeightLater = false;
+
         public AbstractRowContainer(
                 final TableSectionElement rowContainerElement) {
             root = rowContainerElement;
@@ -2115,14 +2117,21 @@ public class Escalator extends Widget
         }
 
         public void autodetectRowHeightLater() {
+            autodetectingRowHeightLater = true;
             Scheduler.get().scheduleFinally(() -> {
                 if (defaultRowHeightShouldBeAutodetected && isAttached()) {
                     autodetectRowHeightNow();
                     defaultRowHeightShouldBeAutodetected = false;
                 }
+                autodetectingRowHeightLater = false;
             });
         }
 
+        @Override
+        public boolean isAutodetectingRowHeightLater() {
+            return autodetectingRowHeightLater;
+        }
+
         private void fireRowHeightChangedEventFinally() {
             if (!rowHeightChangedEventFired) {
                 rowHeightChangedEventFired = true;
diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index e6efe4cbc0..f42312c1d6 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -76,10 +76,7 @@ import com.google.gwt.user.client.ui.MenuItem;
 import com.google.gwt.user.client.ui.PopupPanel;
 import com.google.gwt.user.client.ui.ResizeComposite;
 import com.google.gwt.user.client.ui.Widget;
-import com.vaadin.client.BrowserInfo;
-import com.vaadin.client.DeferredWorker;
-import com.vaadin.client.Focusable;
-import com.vaadin.client.WidgetUtil;
+import com.vaadin.client.*;
 import com.vaadin.client.WidgetUtil.Reference;
 import com.vaadin.client.data.DataChangeHandler;
 import com.vaadin.client.data.DataSource;
@@ -3342,6 +3339,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      */
     private class AutoColumnWidthsRecalculator {
         private double lastCalculatedInnerWidth = -1;
+        private double lastCalculatedInnerHeight = -1;
 
         private final ScheduledCommand calculateCommand = new ScheduledCommand() {
 
@@ -3436,6 +3434,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
             // Update latest width to prevent recalculate on height change.
             lastCalculatedInnerWidth = escalator.getInnerWidth();
+            lastCalculatedInnerHeight = getEscalatorInnerHeight();
         }
 
         private boolean columnsAreGuaranteedToBeWiderThanGrid() {
@@ -9148,6 +9147,18 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 recalculateColumnWidths();
             }
 
+
+            if (getEscalatorInnerHeight() != autoColumnWidthsRecalculator.lastCalculatedInnerHeight) {
+                Scheduler.get().scheduleFinally(() -> {
+                    // Trigger re-calculation of all row positions.
+                    RowContainer.BodyRowContainer body = getEscalator()
+                            .getBody();
+                    if (!body.isAutodetectingRowHeightLater()) {
+                        body.setDefaultRowHeight(body.getDefaultRowHeight());
+                    }
+                });
+            }
+
             // Vertical resizing could make editor positioning invalid so it
             // needs to be recalculated on resize
             if (isEditorActive()) {
@@ -9161,6 +9172,11 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         });
     }
 
+    private double getEscalatorInnerHeight() {
+        return new ComputedStyle(getEscalator().getTableWrapper())
+                .getHeightIncludingBorderPadding();
+    }
+
     /**
      * Grid does not support adding Widgets this way.
      * <p>
diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java
index 5e3ac96efd..c9c5dfb903 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java
@@ -221,6 +221,22 @@ public interface RowContainer {
     public int getRowCount();
 
     /**
+     * This method calculates the current row count directly from the DOM.
+     * <p>
+     * While the container is stable, this value should equal to
+     * {@link #getRowCount()}, but while row counts are being updated, these two
+     * values might differ for a short while.
+     * <p>
+     * Any extra content, such as spacers for the body, should not be included
+     * in this count.
+     *
+     * @since
+     *
+     * @return the actual DOM count of rows
+     */
+    public int getDomRowCount();
+
+    /**
      * The default height of the rows in this RowContainer.
      *
      * @param px
diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java
index 70689b2482..e50682f2c1 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java
@@ -1291,22 +1291,6 @@ public class Escalator extends Widget
         }
 
         /**
-         * This method calculates the current row count directly from the DOM.
-         * <p>
-         * While Escalator is stable, this value should equal to
-         * {@link #getRowCount()}, but while row counts are being updated, these
-         * two values might differ for a short while.
-         * <p>
-         * Any extra content, such as spacers for the body, should not be
-         * included in this count.
-         *
-         * @since 7.5.0
-         *
-         * @return the actual DOM count of rows
-         */
-        public abstract int getDomRowCount();
-
-        /**
          * {@inheritDoc}
          * <p>
          * <em>Implementation detail:</em> This method does no DOM modifications
diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
index 5ab014d413..8af5818eb1 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
@@ -3936,7 +3936,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
         private void setHeightToHeaderCellHeight() {
             RowContainer header = grid.escalator.getHeader();
-            if (header.getRowCount() == 0
+            if (header.getDomRowCount() == 0
                     || !header.getRowElement(0).hasChildNodes()) {
                 getLogger().info(
                         "No header cell available when calculating sidebar button height");
diff --git a/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss b/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss
index 895d9ab975..b30c3b3641 100644
--- a/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss
+++ b/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss
@@ -85,6 +85,10 @@
       top: 0;
       left: 0;
     }
+
+    .#{$primaryStyleName}-header > .#{$primaryStyleName}-row {
+      position: relative;
+    }
   }
 
   .#{$primaryStyleName}-row {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java
new file mode 100644
index 0000000000..b34a0dee77
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java
@@ -0,0 +1,74 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Component;
+import com.vaadin.v7.event.ItemClickEvent;
+import com.vaadin.v7.event.ItemClickEvent.ItemClickListener;
+import com.vaadin.v7.ui.Grid;
+import com.vaadin.v7.ui.Grid.RowReference;
+
+@SuppressWarnings("deprecation")
+public class CompatibilityGridInDetailsRow extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Grid fg = new Grid();
+        fg.setId("grid1");
+        fg.setSizeFull();
+        fg.addColumn("col1", String.class);
+        fg.addColumn("col2", String.class);
+        fg.addRow("Temp 1", "Temp 2");
+        fg.addRow("Temp 3", "Temp 4");
+        fg.setDetailsGenerator(new Grid.DetailsGenerator() {
+            @Override
+            public Component getDetails(RowReference rowReference) {
+                Grid gd = new Grid();
+                gd.setId("grid2");
+                gd.setSizeFull();
+                gd.addHeaderRowAt(0);
+                gd.addColumn("Column 1", String.class);
+                gd.addColumn("Column 2", String.class);
+                gd.getColumn("Column 2").setHidable(true);
+                gd.addColumn("Column 3", String.class);
+                gd.addColumn("Column 4", String.class);
+                gd.addColumn("id", Integer.class);
+                gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus",
+                        "Nicolaus Copernicus", "Nicolaus Copernicus", 1543);
+                gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus",
+                        "Nicolaus Copernicus", "Nicolaus Copernicus", 1543);
+                gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus",
+                        "Nicolaus Copernicus", "Nicolaus Copernicus", 1543);
+                gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus",
+                        "Nicolaus Copernicus", "Nicolaus Copernicus", 1543);
+
+                return gd;
+            }
+        });
+
+        fg.addItemClickListener(new ItemClickListener() {
+            @Override
+            public void itemClick(ItemClickEvent event) {
+                if (event.isDoubleClick()) {
+                    Object itemId = event.getItemId();
+                    fg.setDetailsVisible(itemId, !fg.isDetailsVisible(itemId));
+                }
+            }
+        });
+
+        getLayout().addComponent(fg);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "A nested Grid with multirow header should display all headers and "
+                + "opening the details row shouldn't cause a client-side exception "
+                + "when the nested Grid has hideable rows.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7674;
+    }
+
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java
new file mode 100644
index 0000000000..72c4cf4660
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java
@@ -0,0 +1,76 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.grid.HeightMode;
+import com.vaadin.tests.data.bean.Person;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.ItemClick;
+import com.vaadin.ui.VerticalLayout;
+import com.vaadin.ui.components.grid.DetailsGenerator;
+import com.vaadin.ui.components.grid.HeaderRow;
+import com.vaadin.ui.components.grid.ItemClickListener;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class GridInDetailsRow extends SimpleGridUI {
+
+    int index = 0;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        getLayout().addComponent(createGrid());
+    }
+
+    @Override
+    protected Grid<Person> createGrid() {
+        Grid<Person> grid = super.createGrid();
+        grid.setId("grid" + index);
+        ++index;
+        grid.setSizeFull();
+        grid.setHeightUndefined();
+        grid.setHeightMode(HeightMode.UNDEFINED);
+
+        HeaderRow hr0 = grid.addHeaderRowAt(0);
+        hr0.getCell(grid.getColumns().get(0)).setText("Name");
+        hr0.getCell(grid.getColumns().get(1)).setText("Age");
+        HeaderRow hr1 = grid.getDefaultHeaderRow();
+        hr1.getCell(grid.getColumns().get(0)).setText("Foo");
+        hr1.getCell(grid.getColumns().get(1)).setText("Bar");
+        grid.getColumns().get(1).setHidable(true);
+
+        grid.setDetailsGenerator(new DetailsGenerator<Person>() {
+            @Override
+            public Component apply(Person t) {
+                VerticalLayout layout = new VerticalLayout();
+                layout.setMargin(true);
+                Grid<Person> gd = createGrid();
+                layout.addComponent(gd);
+                return layout;
+            }
+        });
+
+        grid.addItemClickListener(new ItemClickListener<Person>() {
+
+            @Override
+            public void itemClick(ItemClick<Person> event) {
+                if (event.getMouseEventDetails().isDoubleClick()) {
+                    Person item = event.getItem();
+                    grid.setDetailsVisible(item, !grid.isDetailsVisible(item));
+                }
+            }
+        });
+        return grid;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "A nested Grid with multirow header should display all headers and "
+                + "the body rows shouldn't get stuck to default row height.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7674;
+    }
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java
index 4a19ed8915..c7aca31f38 100644
--- a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java
+++ b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java
@@ -172,6 +172,11 @@ public class EscalatorProxy extends Escalator {
         }
 
         @Override
+        public boolean isAutodetectingRowHeightLater() {
+            return rowContainer.isAutodetectingRowHeightLater();
+        }
+
+        @Override
         public void setDefaultRowHeight(double px)
                 throws IllegalArgumentException {
             rowContainer.setDefaultRowHeight(px);
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java
new file mode 100644
index 0000000000..bc9b72f9f7
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java
@@ -0,0 +1,73 @@
+package com.vaadin.tests.components.grid;
+
+import static org.hamcrest.Matchers.greaterThan;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.testbench.elements.GridElement.GridRowElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class CompatibilityGridInDetailsRowTest extends MultiBrowserTest {
+    @Test
+    public void testNestedGridMultiRowHeaderPositions() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        GridRowElement row = grid.getRow(1);
+        row.doubleClick();
+
+        waitForElementPresent(By.className("v-grid-spacer"));
+
+        GridElement nestedGrid = $(GridElement.class).id("grid2");
+        assertEquals("Incorrect header row count.", 2,
+                nestedGrid.getHeaderCount());
+        GridCellElement headerCell00 = nestedGrid.getHeaderCell(0, 0);
+        GridCellElement headerCell11 = nestedGrid.getHeaderCell(1, 1);
+
+        assertThat("Incorrect X-position.", headerCell11.getLocation().getX(),
+                greaterThan(headerCell00.getLocation().getX()));
+        assertThat("Incorrect Y-position.", headerCell11.getLocation().getY(),
+                greaterThan(headerCell00.getLocation().getY()));
+    }
+
+    @Test
+    public void testNestedGridRowHeights() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        GridRowElement row = grid.getRow(1);
+        row.doubleClick();
+
+        waitForElementPresent(By.className("v-grid-spacer"));
+
+        GridElement nestedGrid = $(GridElement.class).id("grid2");
+        grid.findElement(By.className("v-grid-sidebar-button")).click();
+
+        assertNotNull(
+                "There are no options for toggling column visibility but there should be.",
+                getColumnHidingToggle(nestedGrid));
+    }
+
+    /**
+     * Returns the first toggle inside the sidebar for hiding a column, or null
+     * if not found.
+     */
+    protected WebElement getColumnHidingToggle(GridElement grid) {
+        WebElement sidebar = findElement(By.className("v-grid-sidebar-popup"));
+        List<WebElement> elements = sidebar
+                .findElements(By.className("column-hiding-toggle"));
+        for (WebElement e : elements) {
+            return e;
+        }
+        return null;
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java
new file mode 100644
index 0000000000..51e2a56d65
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java
@@ -0,0 +1,51 @@
+package com.vaadin.tests.components.grid;
+
+import static org.hamcrest.Matchers.greaterThan;
+import static org.junit.Assert.assertThat;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.testbench.elements.GridElement.GridRowElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridInDetailsRowTest extends MultiBrowserTest {
+    @Test
+    public void testNestedGridMultiRowHeaderPositions() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        GridRowElement row = grid.getRow(2);
+        row.doubleClick();
+
+        waitForElementPresent(By.className("v-grid-spacer"));
+
+        GridElement nestedGrid = $(GridElement.class).id("grid1");
+        GridCellElement headerCell00 = nestedGrid.getHeaderCell(0, 0);
+        GridCellElement headerCell11 = nestedGrid.getHeaderCell(1, 1);
+
+        assertThat("Incorrect X-position.", headerCell11.getLocation().getX(),
+                greaterThan(headerCell00.getLocation().getX()));
+        assertThat("Incorrect Y-position.", headerCell11.getLocation().getY(),
+                greaterThan(headerCell00.getLocation().getY()));
+    }
+
+    @Test
+    public void testNestedGridRowHeights() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        GridRowElement row = grid.getRow(2);
+        row.doubleClick();
+
+        waitForElementPresent(By.className("v-grid-spacer"));
+
+        GridElement nestedGrid = $(GridElement.class).id("grid1");
+        GridCellElement cell = nestedGrid.getCell(0, 0);
+
+        assertThat("Incorrect row height.", cell.getSize().height,
+                greaterThan(30));
+    }
+}