Fixing Grid Layout required indicator position (#18418)

Required indicators in Grid Layout are now located right after the fields

Change-Id: I764fe15a967673c3a70a2a8ab97e7d1a223061fa

3 files changed, 266 insertions, 22 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/layout/VLayoutSlot.java b/client/src/main/java/com/vaadin/client/ui/layout/VLayoutSlot.java
index bc0c6739bb..5260e1866e 100644
--- a/client/src/main/java/com/vaadin/client/ui/layout/VLayoutSlot.java
+++ b/client/src/main/java/com/vaadin/client/ui/layout/VLayoutSlot.java
@@ -87,24 +87,21 @@ public abstract class VLayoutSlot {
                 : null;
         int captionWidth = getCaptionWidth();
 
-        boolean captionAboveCompnent;
+        boolean captionAboveComponent;
         if (caption == null) {
-            captionAboveCompnent = false;
-            style.clearPaddingRight();
+            captionAboveComponent = false;
         } else {
-            captionAboveCompnent = !caption.shouldBePlacedAfterComponent();
-            if (!captionAboveCompnent) {
+            captionAboveComponent = !caption.shouldBePlacedAfterComponent();
+            if (!captionAboveComponent) {
                 availableWidth -= captionWidth;
                 if (availableWidth < 0) {
                     availableWidth = 0;
                 }
-                captionStyle.clearLeft();
-                captionStyle.setRight(0, Unit.PX);
+
                 style.setPaddingRight(captionWidth, Unit.PX);
+                widget.getElement().getStyle().setPosition(Position.RELATIVE);
             } else {
                 captionStyle.setLeft(0, Unit.PX);
-                captionStyle.clearRight();
-                style.clearPaddingRight();
             }
         }
 
@@ -125,34 +122,36 @@ public abstract class VLayoutSlot {
             reportActualRelativeWidth(Math.round((float) allocatedContentWidth));
         }
 
+        double usedWidth; // widget width in px
+        if (isRelativeWidth()) {
+            usedWidth = allocatedContentWidth;
+        } else {
+            usedWidth = getWidgetWidth();
+        }
+
         style.setLeft(Math.round(currentLocation), Unit.PX);
         AlignmentInfo alignment = getAlignment();
         if (!alignment.isLeft()) {
-            double usedWidth;
-            if (isRelativeWidth()) {
-                usedWidth = allocatedContentWidth;
-            } else {
-                usedWidth = getWidgetWidth();
-            }
-
-            double padding = (allocatedSpace - usedWidth);
+            double padding = availableWidth - usedWidth;
             if (alignment.isHorizontalCenter()) {
                 padding = padding / 2;
             }
 
             long roundedPadding = Math.round(padding);
-            if (captionAboveCompnent) {
-                captionStyle.setLeft(roundedPadding, Unit.PX);
+            if (captionStyle != null) {
+                captionStyle.setLeft(captionAboveComponent ? roundedPadding
+                        : roundedPadding + usedWidth, Unit.PX);
             }
             widget.getElement().getStyle().setLeft(roundedPadding, Unit.PX);
         } else {
-            if (captionAboveCompnent) {
-                captionStyle.setLeft(0, Unit.PX);
+            if (captionStyle != null) {
+                captionStyle.setLeft(captionAboveComponent ? 0 : usedWidth,
+                        Unit.PX);
             }
+
             // Reset left when changing back to align left
             widget.getElement().getStyle().clearLeft();
         }
-
     }
 
     private double parsePercent(String size) {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocation.java b/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocation.java
new file mode 100644
index 0000000000..e432e7b674
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocation.java
@@ -0,0 +1,139 @@
+/*
+ * 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 com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Alignment;
+import com.vaadin.ui.GridLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.VerticalLayout;
+
+/**
+ * Test for grid required indicator location within slots.
+ */
+public class GridLayoutRequiredIndicatorLocation extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        getPage().getCurrent().getStyles()
+                .add(".allow-overflow { overflow: visible; }");
+        getPage().getCurrent().getStyles()
+                .add(".colored { background: lime; overflow: visible; }");
+        getPage().getCurrent().getStyles()
+                .add(".pink { background: pink; overflow: visible; }");
+        getPage().getCurrent().getStyles().add(
+                ".v-gridlayout-slot { border: 1px solid red; }");
+
+        GridLayout rootLayout = new GridLayout(2, 2);
+        rootLayout.addStyleName("allow-overflow");
+        rootLayout.setSpacing(true);
+        addComponent(rootLayout);
+
+        GridLayout gridLayout = createGridLayout(false);
+        gridLayout.addStyleName("allow-overflow");
+        gridLayout.addStyleName("colored");
+        rootLayout.addComponent(gridLayout);
+
+        // for reference, VerticalLayout does it right
+        VerticalLayout vl = createVerticalLayout(false);
+        vl.addStyleName("allow-overflow");
+        vl.addStyleName("colored");
+        rootLayout.addComponent(vl);
+
+        GridLayout gridLayout2 = createGridLayout(true);
+        gridLayout2.addStyleName("allow-overflow");
+        gridLayout2.addStyleName("colored");
+        rootLayout.addComponent(gridLayout2);
+
+        VerticalLayout vl2 = createVerticalLayout(true);
+        vl2.addStyleName("allow-overflow");
+        vl2.addStyleName("colored");
+        rootLayout.addComponent(vl2);
+    }
+
+    private VerticalLayout createVerticalLayout(boolean useCaption) {
+        VerticalLayout vl = new VerticalLayout();
+        vl.setWidth("320px");
+
+        addLabel(vl, "200px", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(vl, "40%", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(vl, "100%", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(vl, "200px", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(vl, "30%", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(vl, "100%", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(vl, "200px", Alignment.MIDDLE_RIGHT, useCaption);
+        addLabel(vl, "50%", Alignment.MIDDLE_RIGHT, useCaption);
+        addLabel(vl, "100%", Alignment.MIDDLE_RIGHT, useCaption);
+        return vl;
+    }
+
+    private GridLayout createGridLayout(boolean useCaption) {
+        GridLayout gridLayout = new GridLayout();
+        gridLayout.setColumns(2);
+        gridLayout.setWidth("500px");
+        gridLayout.setColumnExpandRatio(0, 0);
+        gridLayout.setColumnExpandRatio(1, 1);
+
+        addLabel(gridLayout, "200px", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(gridLayout, "40%", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(gridLayout, "100%", Alignment.MIDDLE_LEFT, useCaption);
+        addLabel(gridLayout, "200px", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(gridLayout, "30%", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(gridLayout, "100%", Alignment.MIDDLE_CENTER, useCaption);
+        addLabel(gridLayout, "200px", Alignment.MIDDLE_RIGHT, useCaption);
+        addLabel(gridLayout, "50%", Alignment.MIDDLE_RIGHT, useCaption);
+        addLabel(gridLayout, "100%", Alignment.MIDDLE_RIGHT, useCaption);
+        return gridLayout;
+    }
+
+    private void addLabel(GridLayout layout, String width, Alignment alignment,
+            boolean useCaption) {
+        Label label = new Label("Align " + alignment.getHorizontalAlignment()
+                + " width " + width);
+        label.setWidth("180px");
+        label.addStyleName("pink");
+        layout.addComponent(label);
+
+        // TODO also test with captions
+        TextField field = new TextField(useCaption ? "caption" : null);
+        field.setRequired(true);
+        field.setWidth(width);
+        layout.addComponent(field);
+        layout.setComponentAlignment(field, alignment);
+    }
+
+    private void addLabel(VerticalLayout layout, String width,
+            Alignment alignment, boolean useCaption) {
+        TextField field = new TextField(useCaption ? "caption" : null);
+        field.setRequired(true);
+        field.setWidth(width);
+        layout.addComponent(field);
+        layout.setComponentAlignment(field, alignment);
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 18418;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "If a GridLayout slot has a size smaller than 100%, the required indicators should be at the end of each field";
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocationTest.java b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocationTest.java
new file mode 100644
index 0000000000..a1afe0edb3
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutRequiredIndicatorLocationTest.java
@@ -0,0 +1,106 @@
+/*
+ * Copyright 2000-2016 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.io.IOException;
+
+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 GridLayoutRequiredIndicatorLocationTest extends MultiBrowserTest {
+
+    private WebElement gridLayoutSlot;
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+        gridLayoutSlot = $(GridLayoutElement.class).first()
+                .findElement(By.className("v-gridlayout-slot"));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationLeftFixedField() {
+        assertIndicatorPosition(getSlot(1));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationLeftRelativeField() {
+        assertIndicatorPosition(getSlot(3));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationLeft100PercentField() {
+        assertIndicatorPosition(getSlot(5));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationCenterFixedField() {
+        assertIndicatorPosition(getSlot(7));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationCenterRelativeField() {
+        assertIndicatorPosition(getSlot(9));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationCenter100PercentField() {
+        assertIndicatorPosition(getSlot(11));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationRightFixedField() {
+        assertIndicatorPosition(getSlot(13));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationRightRelativeField() {
+        assertIndicatorPosition(getSlot(15));
+    }
+
+    @Test
+    public void testRequiredIndicatorLocationRight100PercentField() {
+        assertIndicatorPosition(getSlot(17));
+    }
+
+    private void assertIndicatorPosition(WebElement slot) {
+        WebElement field = slot.findElement(By.tagName("input"));
+        WebElement caption = slot.findElement(By.className("v-caption"));
+
+        int desiredIndicatorPosition = field.getLocation().getX()
+                + field.getSize().getWidth();
+        int actualIndicatorPosition = caption.getLocation().getX();
+
+        Assert.assertEquals("Required indicator has wrong position",
+                desiredIndicatorPosition, actualIndicatorPosition, 1d);
+    }
+
+    @Test
+    public void testScreenshotMatches() throws IOException {
+        compareScreen("indicators");
+    }
+
+    private WebElement getSlot(int index) {
+        return gridLayoutSlot.findElements(By.className("v-gridlayout-slot"))
+                .get(index);
+    }
+}