Fix GridLayout spacing if first row/column empty and set to be hidden (#10593)

2 files changed, 26 insertions, 11 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VGridLayout.java b/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
index b7adf70c2c..d85a0bd0d3 100644
--- a/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
+++ b/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
@@ -228,11 +228,19 @@ public class VGridLayout extends ComplexPanel {
     }
 
     private int calcRowUsedSpace() {
-        int usedSpace = minRowHeights[0];
+        int usedSpace = 0;
         int verticalSpacing = getVerticalSpacing();
-        for (int i = 1; i < minRowHeights.length; i++) {
+        boolean visibleFound = false;
+        for (int i = 0; i < minRowHeights.length; i++) {
             if (minRowHeights[i] > 0 || !hiddenEmptyRow(i)) {
-                usedSpace += verticalSpacing + minRowHeights[i];
+                if (visibleFound) {
+                    // only include spacing if there already is a visible row
+                    // before this one
+                    usedSpace += verticalSpacing + minRowHeights[i];
+                } else {
+                    usedSpace += minRowHeights[i];
+                    visibleFound = true;
+                }
             }
         }
         return usedSpace;
@@ -289,11 +297,19 @@ public class VGridLayout extends ComplexPanel {
      * Calculates column used space
      */
     private int calcColumnUsedSpace() {
-        int usedSpace = minColumnWidths[0];
+        int usedSpace = 0;
         int horizontalSpacing = getHorizontalSpacing();
-        for (int i = 1; i < minColumnWidths.length; i++) {
+        boolean visibleFound = false;
+        for (int i = 0; i < minColumnWidths.length; i++) {
             if (minColumnWidths[i] > 0 || !hiddenEmptyColumn(i)) {
-                usedSpace += horizontalSpacing + minColumnWidths[i];
+                if (visibleFound) {
+                    // only include spacing if there already is a visible column
+                    // before this one
+                    usedSpace += horizontalSpacing + minColumnWidths[i];
+                } else {
+                    usedSpace += minColumnWidths[i];
+                    visibleFound = true;
+                }
             }
         }
         return usedSpace;
diff --git a/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExtraSpacingTest.java b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExtraSpacingTest.java
index 15b4f9c507..22cbddd4c4 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExtraSpacingTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExtraSpacingTest.java
@@ -49,6 +49,7 @@ public class GridLayoutExtraSpacingTest extends MultiBrowserTest {
         // Spacing off, not hiding empty rows/columns
         // There should not be any spacing (red) above the csslayout
         verifySpacingAbove(0, gridLayout, component);
+        verifySpacingBelow(0, gridLayout, component);
 
         CheckBoxElement hideRowsColumnsCheckbox = $(CheckBoxElement.class)
                 .caption("hide empty rows/columns").first();
@@ -57,16 +58,14 @@ public class GridLayoutExtraSpacingTest extends MultiBrowserTest {
         // Spacing off, hiding empty rows/columns
         // There should not be any spacing (red) above the csslayout
         verifySpacingAbove(0, gridLayout, component);
+        verifySpacingBelow(0, gridLayout, component);
 
         check(spacingCheckbox);
         // Spacing on, hiding empty rows/columns
         // There should not be any spacing (red) above or below the csslayout
 
-        // Oh PhantomJs...
-        sleep(100);
-        // FIXME: This should be 0 but there is a bug somewhere
-        // verifySpacingAbove(0, gridLayout, component);
-        verifySpacingBelow(6, gridLayout, component);
+        verifySpacingAbove(0, gridLayout, component);
+        verifySpacingBelow(0, gridLayout, component);
 
     }