Grid layout slots should be slot size not slot content size (#17039)

Change-Id: I73a26b0d2f07bb624df6037e4e77f7137fd21791

5 files changed, 201 insertions, 20 deletions

diff --git a/client/src/com/vaadin/client/ui/VGridLayout.java b/client/src/com/vaadin/client/ui/VGridLayout.java
index 90ae944322..d1e055eb5d 100644
--- a/client/src/com/vaadin/client/ui/VGridLayout.java
+++ b/client/src/com/vaadin/client/ui/VGridLayout.java
@@ -889,6 +889,10 @@ public class VGridLayout extends ComplexPanel {
                 cell.slot.setCaption(null);
                 cell.slot.getWrapperElement().removeFromParent();
                 cell.slot = null;
+                Style style = w.getElement().getStyle();
+                style.clearTop();
+                style.clearLeft();
+                style.clearPosition();
 
                 if (cells.length < cell.col && cells.length != 0
                         && cells[0].length < cell.row
diff --git a/client/src/com/vaadin/client/ui/VSlider.java b/client/src/com/vaadin/client/ui/VSlider.java
index f5769ddf74..651880502c 100644
--- a/client/src/com/vaadin/client/ui/VSlider.java
+++ b/client/src/com/vaadin/client/ui/VSlider.java
@@ -201,7 +201,7 @@ public class VSlider extends SimpleFocusablePanel implements Field,
          * applied to call code for parentElement only in case it exists.
          */
         if (getElement().hasParentElement()) {
-            final Element p = getElement().getParentElement();
+            final Element p = getElement();
             if (p.getPropertyInt(domProperty) > MIN_SIZE) {
                 if (isVertical()) {
                     setHeight();
@@ -216,8 +216,10 @@ public class VSlider extends SimpleFocusablePanel implements Field,
 
                     @Override
                     public void execute() {
-                        final Element p = getElement().getParentElement();
-                        if (p.getPropertyInt(domProperty) > (MIN_SIZE + 5)) {
+                        final Element p = getElement();
+                        if (p.getPropertyInt(domProperty) > (MIN_SIZE + 5)
+                                || propertyNotNullOrEmpty(styleAttribute,
+                                        p)) {
                             if (isVertical()) {
                                 setHeight();
                             } else {
@@ -227,6 +229,14 @@ public class VSlider extends SimpleFocusablePanel implements Field,
                             setValue(value, false);
                         }
                     }
+
+                    // Style has non empty property
+                    private boolean propertyNotNullOrEmpty(
+                            final String styleAttribute, final Element p) {
+                        return p.getStyle().getProperty(styleAttribute) != null
+                                && !p.getStyle().getProperty(styleAttribute)
+                                        .isEmpty();
+                    }
                 });
             }
         }
diff --git a/client/src/com/vaadin/client/ui/layout/VLayoutSlot.java b/client/src/com/vaadin/client/ui/layout/VLayoutSlot.java
index 50020e51fe..5d463712df 100644
--- a/client/src/com/vaadin/client/ui/layout/VLayoutSlot.java
+++ b/client/src/com/vaadin/client/ui/layout/VLayoutSlot.java
@@ -69,6 +69,11 @@ public abstract class VLayoutSlot {
 
     public void setAlignment(AlignmentInfo alignment) {
         this.alignment = alignment;
+        // if alignment is something other than topLeft then we need to align
+        // the component inside this slot
+        if (alignment != null && (!alignment.isLeft() || !alignment.isTop())) {
+            widget.getElement().getStyle().setPosition(Position.ABSOLUTE);
+        }
     }
 
     public void positionHorizontally(double currentLocation,
@@ -109,11 +114,7 @@ public abstract class VLayoutSlot {
             style.clearMarginRight();
         }
 
-        if (isRelativeWidth()) {
-            style.setPropertyPx("width", (int) availableWidth);
-        } else {
-            style.clearProperty("width");
-        }
+        style.setPropertyPx("width", (int) availableWidth);
 
         double allocatedContentWidth = 0;
         if (isRelativeWidth()) {
@@ -124,6 +125,8 @@ public abstract class VLayoutSlot {
             reportActualRelativeWidth(Math.round((float) allocatedContentWidth));
         }
 
+        style.setLeft(Math.round(currentLocation), Unit.PX);
+        double padding = 0;
         AlignmentInfo alignment = getAlignment();
         if (!alignment.isLeft()) {
             double usedWidth;
@@ -133,25 +136,26 @@ public abstract class VLayoutSlot {
                 usedWidth = getWidgetWidth();
             }
             if (alignment.isHorizontalCenter()) {
-                currentLocation += (allocatedSpace - usedWidth) / 2d;
+                padding = (allocatedSpace - usedWidth) / 2d;
                 if (captionAboveCompnent) {
                     captionStyle.setLeft(
                             Math.round(usedWidth - captionWidth) / 2, Unit.PX);
                 }
             } else {
-                currentLocation += (allocatedSpace - usedWidth);
+                padding = (allocatedSpace - usedWidth);
                 if (captionAboveCompnent) {
                     captionStyle.setLeft(Math.round(usedWidth - captionWidth),
                             Unit.PX);
                 }
             }
+            widget.getElement().getStyle()
+                    .setLeft(Math.round(padding), Unit.PX);
         } else {
             if (captionAboveCompnent) {
                 captionStyle.setLeft(0, Unit.PX);
             }
         }
 
-        style.setLeft(Math.round(currentLocation), Unit.PX);
     }
 
     private double parsePercent(String size) {
@@ -184,11 +188,7 @@ public abstract class VLayoutSlot {
             style.clearMarginBottom();
         }
 
-        if (isRelativeHeight()) {
-            style.setHeight(contentHeight, Unit.PX);
-        } else {
-            style.clearHeight();
-        }
+        style.setHeight(contentHeight, Unit.PX);
 
         double allocatedContentHeight = 0;
         if (isRelativeHeight()) {
@@ -199,6 +199,8 @@ public abstract class VLayoutSlot {
                     .round((float) allocatedContentHeight));
         }
 
+        style.setTop(currentLocation, Unit.PX);
+        double padding = 0;
         AlignmentInfo alignment = getAlignment();
         if (!alignment.isTop()) {
             double usedHeight;
@@ -208,13 +210,14 @@ public abstract class VLayoutSlot {
                 usedHeight = getUsedHeight();
             }
             if (alignment.isVerticalCenter()) {
-                currentLocation += (allocatedSpace - usedHeight) / 2d;
+                padding = (allocatedSpace - usedHeight) / 2d;
             } else {
-                currentLocation += (allocatedSpace - usedHeight);
+                padding = (allocatedSpace - usedHeight);
             }
-        }
+            padding += captionHeight;
 
-        style.setTop(currentLocation, Unit.PX);
+            widget.getElement().getStyle().setTop(padding, Unit.PX);
+        }
     }
 
     protected void reportActualRelativeHeight(int allocatedHeight) {
diff --git a/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUI.java b/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUI.java
new file mode 100644
index 0000000000..d2ca7700ad
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUI.java
@@ -0,0 +1,53 @@
+package com.vaadin.tests.components.gridlayout;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.datefield.Resolution;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.GridLayout;
+import com.vaadin.ui.InlineDateField;
+import com.vaadin.ui.Label;
+
+@SuppressWarnings("serial")
+public class GridLayoutCellSizesUI extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        // Create a 4 by 4 grid layout
+        final GridLayout grid = new GridLayout(4, 4);
+
+        // Fill out the first row using the cursor
+        grid.addComponent(new Button("R/C 1"));
+        for (int i = 0; i < 3; i++) {
+            grid.addComponent(new Button("Col " + (grid.getCursorX() + 1)));
+        }
+
+        // Fill out the first column using coordinates
+        for (int i = 1; i < 4; i++) {
+            grid.addComponent(new Button("Row " + i), 0, i);
+        }
+
+        // Add some components of various shapes.
+        grid.addComponent(new Button("3x1 button"), 1, 1, 3, 1);
+        grid.addComponent(new Label("1x2 cell"), 1, 2, 1, 3);
+        final InlineDateField date = new InlineDateField("A 2x2 date field");
+        date.setResolution(Resolution.DAY);
+        grid.addComponent(date, 2, 2, 3, 3);
+
+        grid.setMargin(true);
+        grid.setSizeUndefined();
+
+        addComponent(grid);
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 17039;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Grid cells should be full size when adding borders around the cells";
+    }
+}
\ No newline at end of file
diff --git a/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUITest.java b/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUITest.java
new file mode 100644
index 0000000000..8c753b1f5c
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/gridlayout/GridLayoutCellSizesUITest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.gridlayout;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.GridLayoutElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridLayoutCellSizesUITest extends MultiBrowserTest {
+
+    private List<WebElement> slots4x4;
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+        slots4x4 = getSlots();
+    }
+
+    @Test
+    public void equalsHeightSlotsShouldHaveTheSameHeight() {
+        // items in positions 0,1,2,3,4,5 should have the same height
+        int firstSlotHeight = getSlotHeight(0);
+        for (int i = 1; i < 6; i++) {
+            Assert.assertEquals("Cell height didn't match for cell: " + i,
+                    firstSlotHeight, getSlotHeight(i));
+        }
+    }
+
+    @Test
+    public void expandedAndLargeSizeSlotsShouldNotEqualFirstSlot() {
+        int firstSlotHeight = getSlotHeight(0);
+
+        assertNotMatchesSmallHeight(firstSlotHeight, 6, "Row 2");
+        assertNotMatchesSmallHeight(firstSlotHeight, 7, "1x2 cell");
+        assertNotMatchesSmallHeight(firstSlotHeight, 8, "A 2x2 date field");
+        assertNotMatchesSmallHeight(firstSlotHeight, 9, "Row 3");
+    }
+
+    @Test
+    public void expandedRowsShouldHaveCorrectHeight() {
+        // Slots expanding over 2 rows should have the same height.
+        Assert.assertEquals("1x2 and 2x2 cell heights didn't match",
+                getSlotHeight(7), getSlotHeight(8));
+
+        // Slots on same row as the 1x2 label should have the same combined
+        // height.
+        Assert.assertEquals(
+                "1x2 and combined row two and row three cell heights didn't match",
+                getSlotHeight(7), getSlotHeight(6) + getSlotHeight(9));
+    }
+
+    @Test
+    public void expandedRowsShouldHaveCorrectWidth() {
+        // Col 2 slot should be the dame width as 1x2 cell slot
+        Assert.assertEquals(
+                "Col 2 slot was not the same width as slot for 1x2 cell",
+                getSlotWidth(1), getSlotWidth(7));
+
+        // Row one col 3 & 4 should be as wide as the 2x2 date field
+        Assert.assertEquals(
+                "2x2 date field width didn't match col 3 & col 4 combined width",
+                getSlotWidth(8), getSlotWidth(2) + getSlotWidth(3));
+
+        // 3x1 button should be as wide as 1x2cell + 2x2 data field
+        Assert.assertEquals(
+                "3x1 slot width wasn't the same as the combined slot widths of 1x2 cell and 2x2 date field",
+                getSlotWidth(5), getSlotWidth(7) + getSlotWidth(8));
+
+    }
+
+    private void assertNotMatchesSmallHeight(int firstSlotHeight, int i,
+            String id) {
+        Assert.assertNotEquals("Big slot '" + id
+                + "' matched small slots in height", firstSlotHeight,
+                getSlotHeight(i));
+    }
+
+    private int getSlotHeight(int slot) {
+        return slots4x4.get(slot).getSize().height;
+    }
+
+    private int getSlotWidth(int slot) {
+        return slots4x4.get(slot).getSize().width;
+    }
+
+    private List<WebElement> getSlots() {
+        GridLayoutElement layout = $(GridLayoutElement.class).first();
+
+        return layout.findElements(By.className("v-gridlayout-slot"));
+    }
+}