Horizontal split panel height shouldn't ignore second component (#15149)

Change-Id: Ie925757167b6d8521707b6d5515157eca85f9dba

3 files changed, 261 insertions, 91 deletions

diff --git a/client/src/com/vaadin/client/ui/VAbstractSplitPanel.java b/client/src/com/vaadin/client/ui/VAbstractSplitPanel.java
index 6ee88d51dd..9d32355b70 100644
--- a/client/src/com/vaadin/client/ui/VAbstractSplitPanel.java
+++ b/client/src/com/vaadin/client/ui/VAbstractSplitPanel.java
@@ -199,6 +199,7 @@ public class VAbstractSplitPanel extends ComplexPanel {
 
     private void setOrientation(Orientation orientation) {
         this.orientation = orientation;
+
         if (orientation == Orientation.HORIZONTAL) {
             splitter.getStyle().setHeight(100, Unit.PCT);
             splitter.getStyle().setTop(0, Unit.PX);
@@ -393,107 +394,118 @@ public class VAbstractSplitPanel extends ComplexPanel {
             return;
         }
 
-        int wholeSize;
-        int pixelPosition;
-
         switch (orientation) {
         case HORIZONTAL:
-            wholeSize = DOM.getElementPropertyInt(wrapper, "clientWidth");
-            pixelPosition = DOM.getElementPropertyInt(splitter, "offsetLeft");
-
-            // reposition splitter in case it is out of box
-            if ((pixelPosition > 0 && pixelPosition + getSplitterSize() > wholeSize)
-                    || (positionReversed && pixelPosition < 0)) {
-                pixelPosition = wholeSize - getSplitterSize();
-                if (pixelPosition < 0) {
-                    pixelPosition = 0;
-                }
-                // Move splitter within bounds, but don't remember the new value
-                setSplitPosition(pixelPosition + "px", false);
-                return;
-            }
+            horizontalOrientationUpdateSizes();
+            break;
+        case VERTICAL:
+            verticalOrientationUpdateSizes();
+            break;
+        }
+    }
 
-            firstContainer.getStyle().setWidth(pixelPosition, Unit.PX);
-            int secondContainerWidth = (wholeSize - pixelPosition - getSplitterSize());
-            if (secondContainerWidth < 0) {
-                secondContainerWidth = 0;
-            }
-            secondContainer.getStyle().setWidth(secondContainerWidth, Unit.PX);
-            secondContainer.getStyle().setLeft(
-                    pixelPosition + getSplitterSize(), Unit.PX);
-
-            LayoutManager layoutManager = LayoutManager.get(client);
-            ConnectorMap connectorMap = ConnectorMap.get(client);
-            if (firstChild != null) {
-                ComponentConnector connector = connectorMap
-                        .getConnector(firstChild);
-                if (connector.isRelativeWidth()) {
-                    layoutManager.reportWidthAssignedToRelative(connector,
-                            pixelPosition);
-                } else {
-                    layoutManager.setNeedsMeasure(connector);
-                }
+    private void verticalOrientationUpdateSizes() {
+        int wholeSize = DOM.getElementPropertyInt(wrapper, "clientHeight");
+        int pixelPosition = DOM.getElementPropertyInt(splitter, "offsetTop");
+
+        // reposition splitter in case it is out of box
+        if ((pixelPosition > 0 && pixelPosition + getSplitterSize() > wholeSize)
+                || (positionReversed && pixelPosition < 0)) {
+            pixelPosition = wholeSize - getSplitterSize();
+            if (pixelPosition < 0) {
+                pixelPosition = 0;
             }
-            if (secondChild != null) {
-                ComponentConnector connector = connectorMap
-                        .getConnector(secondChild);
-                if (connector.isRelativeWidth()) {
-                    layoutManager.reportWidthAssignedToRelative(connector,
-                            secondContainerWidth);
-                } else {
-                    layoutManager.setNeedsMeasure(connector);
-                }
+            // Move splitter within bounds, but don't remember the new value
+            setSplitPosition(pixelPosition + "px", false);
+            return;
+        }
+
+        firstContainer.getStyle().setHeight(pixelPosition, Unit.PX);
+        int secondContainerHeight = (wholeSize - pixelPosition - getSplitterSize());
+        if (secondContainerHeight < 0) {
+            secondContainerHeight = 0;
+        }
+        secondContainer.getStyle().setHeight(secondContainerHeight, Unit.PX);
+        secondContainer.getStyle().setTop(pixelPosition + getSplitterSize(),
+                Unit.PX);
+
+        LayoutManager layoutManager = LayoutManager.get(client);
+        ConnectorMap connectorMap = ConnectorMap.get(client);
+        if (firstChild != null) {
+            ComponentConnector connector = connectorMap
+                    .getConnector(firstChild);
+            if (connector.isRelativeHeight()) {
+                layoutManager.reportHeightAssignedToRelative(connector,
+                        pixelPosition);
+            } else {
+                layoutManager.setNeedsMeasure(connector);
             }
-            break;
-        case VERTICAL:
-            wholeSize = DOM.getElementPropertyInt(wrapper, "clientHeight");
-            pixelPosition = DOM.getElementPropertyInt(splitter, "offsetTop");
-
-            // reposition splitter in case it is out of box
-            if ((pixelPosition > 0 && pixelPosition + getSplitterSize() > wholeSize)
-                    || (positionReversed && pixelPosition < 0)) {
-                pixelPosition = wholeSize - getSplitterSize();
-                if (pixelPosition < 0) {
-                    pixelPosition = 0;
-                }
-                // Move splitter within bounds, but don't remember the new value
-                setSplitPosition(pixelPosition + "px", false);
-                return;
+        }
+        if (secondChild != null) {
+            ComponentConnector connector = connectorMap
+                    .getConnector(secondChild);
+            if (connector.isRelativeHeight()) {
+                layoutManager.reportHeightAssignedToRelative(connector,
+                        secondContainerHeight);
+            } else {
+                layoutManager.setNeedsMeasure(connector);
             }
+        }
+    }
+
+    private void horizontalOrientationUpdateSizes() {
+        int wholeSize = DOM.getElementPropertyInt(wrapper, "clientWidth");
+        int pixelPosition = DOM.getElementPropertyInt(splitter, "offsetLeft");
 
-            firstContainer.getStyle().setHeight(pixelPosition, Unit.PX);
-            int secondContainerHeight = (wholeSize - pixelPosition - getSplitterSize());
-            if (secondContainerHeight < 0) {
-                secondContainerHeight = 0;
+        // reposition splitter in case it is out of box
+        if ((pixelPosition > 0 && pixelPosition + getSplitterSize() > wholeSize)
+                || (positionReversed && pixelPosition < 0)) {
+            pixelPosition = wholeSize - getSplitterSize();
+            if (pixelPosition < 0) {
+                pixelPosition = 0;
             }
-            secondContainer.getStyle()
-                    .setHeight(secondContainerHeight, Unit.PX);
-            secondContainer.getStyle().setTop(
-                    pixelPosition + getSplitterSize(), Unit.PX);
-
-            layoutManager = LayoutManager.get(client);
-            connectorMap = ConnectorMap.get(client);
-            if (firstChild != null) {
-                ComponentConnector connector = connectorMap
-                        .getConnector(firstChild);
-                if (connector.isRelativeHeight()) {
-                    layoutManager.reportHeightAssignedToRelative(connector,
-                            pixelPosition);
-                } else {
-                    layoutManager.setNeedsMeasure(connector);
-                }
+            // Move splitter within bounds, but don't remember the new value
+            setSplitPosition(pixelPosition + "px", false);
+            return;
+        }
+
+        firstContainer.getStyle().setWidth(pixelPosition, Unit.PX);
+        int secondContainerWidth = (wholeSize - pixelPosition - getSplitterSize());
+        if (secondContainerWidth < 0) {
+            secondContainerWidth = 0;
+        }
+        secondContainer.getStyle().setWidth(secondContainerWidth, Unit.PX);
+        secondContainer.getStyle().setLeft(pixelPosition + getSplitterSize(),
+                Unit.PX);
+
+        LayoutManager layoutManager = LayoutManager.get(client);
+        ConnectorMap connectorMap = ConnectorMap.get(client);
+        if (firstChild != null) {
+            ComponentConnector connector = connectorMap
+                    .getConnector(firstChild);
+            if (connector.isRelativeWidth()) {
+                layoutManager.reportWidthAssignedToRelative(connector,
+                        pixelPosition);
+            } else {
+                layoutManager.setNeedsMeasure(connector);
             }
-            if (secondChild != null) {
-                ComponentConnector connector = connectorMap
-                        .getConnector(secondChild);
-                if (connector.isRelativeHeight()) {
-                    layoutManager.reportHeightAssignedToRelative(connector,
-                            secondContainerHeight);
-                } else {
-                    layoutManager.setNeedsMeasure(connector);
-                }
+        }
+        if (secondChild != null) {
+            ComponentConnector connector = connectorMap
+                    .getConnector(secondChild);
+            if (connector.isRelativeWidth()) {
+                layoutManager.reportWidthAssignedToRelative(connector,
+                        secondContainerWidth);
+            } else {
+                layoutManager.setNeedsMeasure(connector);
             }
-            break;
+        }
+
+        // previous layout pass may have changed the position already, needs to
+        // be reset before calculating which positioning should be used
+        secondContainer.getStyle().setPosition(Position.ABSOLUTE);
+        if (getOffsetHeight() == 0) {
+            secondContainer.getStyle().setPosition(Position.RELATIVE);
         }
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeight.java b/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeight.java
new file mode 100644
index 0000000000..4da5430cb1
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeight.java
@@ -0,0 +1,87 @@
+/*
+ * 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.splitpanel;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.MarginInfo;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.HorizontalSplitPanel;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.VerticalSplitPanel;
+
+/**
+ * Test UI for horizontal split panel height.
+ * 
+ * @author Vaadin Ltd
+ */
+public class HorizontalSplitPanelHeight extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        getLayout().setMargin(new MarginInfo(true, false, false, false));
+
+        HorizontalSplitPanel panel = new HorizontalSplitPanel();
+        panel.setCaption("Horizontal 1 - no first component, label as second component");
+        panel.setId("Horizontal 1");
+        Label label = new Label("Label");
+        label.addStyleName("target");
+        panel.setSecondComponent(label);
+
+        addComponent(panel);
+
+        panel = new HorizontalSplitPanel();
+        panel.setCaption("Horizontal 2 - button as first component, label as second component");
+        panel.setId("Horizontal 2");
+        label = new Label("Label");
+        label.addStyleName("target");
+        panel.setSecondComponent(label);
+        panel.setFirstComponent(new Button("button"));
+
+        addComponent(panel);
+
+        panel = new HorizontalSplitPanel();
+        panel.setCaption("Horizontal 3 - fixed height, no first component, label as second component");
+        panel.setId("Horizontal 3");
+        label = new Label("Label");
+        label.addStyleName("target");
+        panel.setSecondComponent(label);
+        panel.setHeight(30, Unit.PIXELS);
+
+        addComponent(panel);
+
+        VerticalSplitPanel vPanel = new VerticalSplitPanel();
+        vPanel.setCaption("Vertical 1 - no first component, label as second component");
+        vPanel.setId("Vertical 1");
+        vPanel.setHeight(100, Unit.PIXELS);
+        Label vLabel = new Label("Label");
+        vLabel.addStyleName("target");
+        vPanel.setSecondComponent(vLabel);
+
+        addComponent(vPanel);
+
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 15149;
+    }
+
+    @Override
+    public String getTestDescription() {
+        return "Height of split panel should be greater than height of second component.";
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeightTest.java b/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeightTest.java
new file mode 100644
index 0000000000..0ceca66dd3
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/splitpanel/HorizontalSplitPanelHeightTest.java
@@ -0,0 +1,71 @@
+/*
+ * 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.splitpanel;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Test for horizontal split panel height in case when only second component is
+ * set.
+ * 
+ * @author Vaadin Ltd
+ */
+public class HorizontalSplitPanelHeightTest extends MultiBrowserTest {
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+
+        openTestURL();
+    }
+
+    @Test
+    public void testHorizontalWithoutFirstComponent() {
+        testSplitPanel("Horizontal 1");
+    }
+
+    @Test
+    public void testHorizontalWithFirstComponent() {
+        testSplitPanel("Horizontal 2");
+    }
+
+    @Test
+    public void testHorizontalWithFixedHeight() {
+        testSplitPanel("Horizontal 3");
+    }
+
+    @Test
+    public void testVerticalWithoutFirstComponent() {
+        testSplitPanel("Vertical 1");
+    }
+
+    private void testSplitPanel(String id) {
+        WebElement splitPanel = findElement(By.id(id));
+        WebElement label = splitPanel.findElement(By.className("target"));
+        Assert.assertTrue(id + ": split panel height ("
+                + splitPanel.getSize().getHeight() + ") is less than "
+                + "height of second component (" + label.getSize().getHeight()
+                + ")", splitPanel.getSize().getHeight() >= label.getSize()
+                .getHeight());
+        Assert.assertEquals("Label text in the second panel is not visible",
+                "Label", label.getText());
+    }
+}