Fixes to BoxLayout based on first round of screenshot tests

10 files changed, 155 insertions, 86 deletions

diff --git a/WebContent/VAADIN/themes/base/boxlayout/boxlayout.css b/WebContent/VAADIN/themes/base/boxlayout/boxlayout.css
index e522989769..0ed0d40888 100644
--- a/WebContent/VAADIN/themes/base/boxlayout/boxlayout.css
+++ b/WebContent/VAADIN/themes/base/boxlayout/boxlayout.css
@@ -6,14 +6,14 @@ TODO
 
 */
 
-.v-boxlayout.v-margin-top 		{padding-top: 		18px;}
-.v-boxlayout.v-margin-right 	{padding-right: 	18px;}
-.v-boxlayout.v-margin-bottom 	{padding-bottom: 	18px;}
-.v-boxlayout.v-margin-left 		{padding-left: 		18px;}
+.v-boxlayout.v-margin-top 		{padding-top: 		12px;}
+.v-boxlayout.v-margin-right 	{padding-right: 	12px;}
+.v-boxlayout.v-margin-bottom 	{padding-bottom: 	12px;}
+.v-boxlayout.v-margin-left 		{padding-left: 		12px;}
 
 .v-spacing {
-	width: 7px;
-	height: 7px;
+	width: 6px;
+	height: 6px;
 }
 
 .v-boxlayout {
@@ -39,6 +39,15 @@ TODO
 	vertical-align: top;
 }
 
+/* Clear any floats inside the slot, to prevent unwanted collapsing */
+.v-slot:after {
+	content: "";
+	display: block;
+	clear: both;
+	height: 0;
+	overflow: hidden;
+}
+
 .v-vertical > .v-slot,
 .v-vertical > .v-expand > .v-slot {
 	display: block;
diff --git a/WebContent/VAADIN/themes/base/button/button.css b/WebContent/VAADIN/themes/base/button/button.css
index 2e14d59a90..8424a3e59f 100644
--- a/WebContent/VAADIN/themes/base/button/button.css
+++ b/WebContent/VAADIN/themes/base/button/button.css
@@ -4,7 +4,7 @@
 .v-button {
 	display: inline-block;
 	zoom: 1;
-	text-align: center;
+	text-align: center !important;
 	text-decoration: none;
 	border: 2px outset #ddd;
 	background: #eee;
@@ -84,7 +84,7 @@
  * NativeButton styles (html button element)
  * -------------------------------------- */
 .v-nativebutton {
-	text-align: center;
+	text-align: center !important;
 	cursor: pointer;
 	white-space: nowrap;
 	margin: 0;
diff --git a/WebContent/VAADIN/themes/base/paintable/paintable.css b/WebContent/VAADIN/themes/base/paintable/paintable.css
index fbe7275113..9d700fd4ee 100644
--- a/WebContent/VAADIN/themes/base/paintable/paintable.css
+++ b/WebContent/VAADIN/themes/base/paintable/paintable.css
@@ -3,4 +3,5 @@
 	-moz-box-sizing: border-box;
 	box-sizing: border-box;
 	text-align: left;
+	display: inline-block;
 }
diff --git a/WebContent/VAADIN/themes/reindeer/layouts/layouts.css b/WebContent/VAADIN/themes/reindeer/layouts/layouts.css
index 528d4b9174..650247bade 100644
--- a/WebContent/VAADIN/themes/reindeer/layouts/layouts.css
+++ b/WebContent/VAADIN/themes/reindeer/layouts/layouts.css
@@ -1,21 +1,25 @@
 .v-orderedlayout-margin-top,
 .v-horizontallayout-margin-top,
-.v-verticallayout-margin-top {
+.v-verticallayout-margin-top,
+.v-boxlayout.v-margin-top {
 	padding-top: 18px;
 }
 .v-orderedlayout-margin-right,
 .v-horizontallayout-margin-right,
-.v-verticallayout-margin-right {
+.v-verticallayout-margin-right,
+.v-boxlayout.v-margin-right {
 	padding-right: 18px;
 }
 .v-orderedlayout-margin-bottom,
 .v-horizontallayout-margin-bottom,
-.v-verticallayout-margin-bottom {
+.v-verticallayout-margin-bottom,
+.v-boxlayout.v-margin-bottom {
 	padding-bottom: 18px;
 }
 .v-orderedlayout-margin-left,
 .v-horizontallayout-margin-left,
-.v-verticallayout-margin-left {
+.v-verticallayout-margin-left,
+.v-boxlayout.v-margin-left {
 	padding-left: 18px;
 }
 .v-orderedlayout-spacing-on,
@@ -24,6 +28,10 @@
 	padding-top: 7px;
 	padding-left: 6px;
 }
+.v-spacing {
+	height: 7px;
+	width: 6px;
+}
 /* Different for historical reasons: previously was inherited directly from Base theme */
 /* TODO unify these values in version 7 */
 .v-gridlayout-margin-top {
diff --git a/WebContent/VAADIN/themes/runo/orderedlayout/orderedlayout.css b/WebContent/VAADIN/themes/runo/orderedlayout/orderedlayout.css
index 306292d843..f70e285729 100644
--- a/WebContent/VAADIN/themes/runo/orderedlayout/orderedlayout.css
+++ b/WebContent/VAADIN/themes/runo/orderedlayout/orderedlayout.css
@@ -1,7 +1,8 @@
 .v-orderedlayout-margin-top,
 .v-horizontallayout-margin-top,
 .v-verticallayout-margin-top,
-.v-csslayout-margin-top {
+.v-csslayout-margin-top,
+.v-boxlayout.v-margin-top {
 	padding-top: 15px;
 }
 .v-orderedlayout-margin-right,
@@ -13,7 +14,8 @@
 .v-orderedlayout-margin-bottom,
 .v-horizontallayout-margin-bottom,
 .v-verticallayout-margin-bottom,
-.v-csslayout-margin-bottom {
+.v-csslayout-margin-bottom,
+.v-boxlayout.v-margin-bottom {
 	padding-bottom: 15px;
 }
 .v-orderedlayout-margin-left,
@@ -28,6 +30,10 @@
 	padding-top: 8px;
 	padding-left: 8px;
 }
+.v-spacing {
+	width: 8px;
+	height: 8px;
+}
 .v-verticallayout-darker,
 .v-horizontallayout-darker,
 .v-gridlayout-darker,
diff --git a/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java b/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
index 5f0ebfda1c..6e9b9af9f7 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
@@ -187,9 +187,10 @@ public abstract class AbstractBoxLayoutConnector extends
         if (child instanceof AbstractFieldConnector) {
             required = ((AbstractFieldConnector) child).isRequired();
         }
+        boolean enabled = child.getState().isEnabled();
         // TODO Description is handled from somewhere else?
 
-        slot.setCaption(caption, iconUrl, styles, error, required);
+        slot.setCaption(caption, iconUrl, styles, error, required, enabled);
 
         slot.setRelativeWidth(child.isRelativeWidth());
         slot.setRelativeHeight(child.isRelativeHeight());
@@ -316,6 +317,8 @@ public abstract class AbstractBoxLayoutConnector extends
                 getLayoutManager().addElementResizeListener(
                         slot.getCaptionElement(), slotCaptionResizeListener);
             } else if (!needsExpand()) {
+                // TODO recheck if removing the listener here breaks anything.
+                // Should be cleaned up.
                 // getLayoutManager().removeElementResizeListener(
                 // slot.getCaptionElement(), slotCaptionResizeListener);
             }
@@ -366,7 +369,9 @@ public abstract class AbstractBoxLayoutConnector extends
     }
 
     private boolean needsExpand() {
-        return hasExpandRatio.size() > 0;
+        boolean canApplyExpand = (getWidget().vertical && !isUndefinedHeight())
+                || !isUndefinedWidth();
+        return hasExpandRatio.size() > 0 && canApplyExpand;
     }
 
     public void preLayout() {
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java b/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
index b102fdb223..9c1fa89109 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
@@ -227,7 +227,8 @@ public class VBoxLayout extends FlowPanel {
         // TODO refactor VCaption and use that instead: creates a tight coupling
         // between this layout and Vaadin, but it's already coupled
         public void setCaption(String captionText, String iconUrl,
-                List<String> styles, String error, boolean required) {
+                List<String> styles, String error, boolean required,
+                boolean enabled) {
 
             // TODO place for optimization: check if any of these have changed
             // since last time, and only run those changes
@@ -278,18 +279,6 @@ public class VBoxLayout extends FlowPanel {
                 icon = null;
             }
 
-            // Error
-            if (error != null) {
-                if (errorIcon == null) {
-                    errorIcon = DOM.createSpan();
-                    errorIcon.setClassName("v-errorindicator");
-                }
-                caption.appendChild(errorIcon);
-            } else if (errorIcon != null) {
-                errorIcon.removeFromParent();
-                errorIcon = null;
-            }
-
             // Required
             if (required) {
                 if (requiredIcon == null) {
@@ -304,8 +293,20 @@ public class VBoxLayout extends FlowPanel {
                 requiredIcon = null;
             }
 
-            // Styles
+            // Error
+            if (error != null) {
+                if (errorIcon == null) {
+                    errorIcon = DOM.createSpan();
+                    errorIcon.setClassName("v-errorindicator");
+                }
+                caption.appendChild(errorIcon);
+            } else if (errorIcon != null) {
+                errorIcon.removeFromParent();
+                errorIcon = null;
+            }
+
             if (caption != null) {
+                // Styles
                 caption.setClassName("v-caption");
 
                 if (styles != null) {
@@ -313,9 +314,14 @@ public class VBoxLayout extends FlowPanel {
                         caption.addClassName("v-caption-" + style);
                     }
                 }
-            }
 
-            if (caption != null) {
+                if (enabled) {
+                    caption.removeClassName("v-disabled");
+                } else {
+                    caption.addClassName("v-disabled");
+                }
+
+                // Caption position
                 if (captionText != null || iconUrl != null) {
                     setCaptionPosition(CaptionPosition.TOP);
                 } else {
@@ -578,67 +584,63 @@ public class VBoxLayout extends FlowPanel {
         }
 
         if (isExpanding) {
-            if (isExpanding) {
-                if (expandWrapper == null) {
-                    expandWrapper = DOM.createDiv();
-                    expandWrapper.setClassName("v-expand");
-                    for (; getElement().getChildCount() > 0;) {
-                        Node el = getElement().getChild(0);
-                        expandWrapper.appendChild(el);
-                    }
-                    getElement().appendChild(expandWrapper);
+            if (expandWrapper == null) {
+                expandWrapper = DOM.createDiv();
+                expandWrapper.setClassName("v-expand");
+                for (; getElement().getChildCount() > 0;) {
+                    Node el = getElement().getChild(0);
+                    expandWrapper.appendChild(el);
                 }
-
-                int totalSize = 0;
-                for (Widget w : getChildren()) {
-                    Slot slot = (Slot) w;
-                    if (slot.getExpandRatio() == -1) {
-                        if (layoutManager != null) {
-                            // TODO check caption position
-                            if (vertical) {
+                getElement().appendChild(expandWrapper);
+            }
+
+            int totalSize = 0;
+            for (Widget w : getChildren()) {
+                Slot slot = (Slot) w;
+                if (slot.getExpandRatio() == -1) {
+                    if (layoutManager != null) {
+                        // TODO check caption position
+                        if (vertical) {
+                            totalSize += layoutManager.getOuterHeight(slot
+                                    .getWidget().getElement())
+                                    - layoutManager.getMarginHeight(slot
+                                            .getWidget().getElement());
+                            if (slot.hasCaption()) {
                                 totalSize += layoutManager.getOuterHeight(slot
-                                        .getWidget().getElement())
+                                        .getCaptionElement())
                                         - layoutManager.getMarginHeight(slot
-                                                .getWidget().getElement());
-                                if (slot.hasCaption()) {
-                                    totalSize += layoutManager
-                                            .getOuterHeight(slot
-                                                    .getCaptionElement())
-                                            - layoutManager
-                                                    .getMarginHeight(slot
-                                                            .getCaptionElement());
-                                }
-                            } else {
-                                totalSize += layoutManager.getOuterWidth(slot
-                                        .getWidget().getElement())
-                                        - layoutManager.getMarginWidth(slot
-                                                .getWidget().getElement());
+                                                .getCaptionElement());
                             }
                         } else {
-                            totalSize += vertical ? slot.getOffsetHeight()
-                                    : slot.getOffsetWidth();
+                            totalSize += layoutManager.getOuterWidth(slot
+                                    .getWidget().getElement())
+                                    - layoutManager.getMarginWidth(slot
+                                            .getWidget().getElement());
                         }
+                    } else {
+                        totalSize += vertical ? slot.getOffsetHeight() : slot
+                                .getOffsetWidth();
                     }
-                    // TODO fails in Opera, always returns 0
-                    totalSize += slot.getSpacingSize(vertical);
                 }
-
-                // When we set the margin to the first child, we don't need
-                // overflow:hidden in the layout root element, since the wrapper
-                // would otherwise be placed outside of the layout root element
-                // and block events on elements below it.
-                if (vertical) {
-                    expandWrapper.getStyle().setPaddingTop(totalSize, Unit.PX);
-                    expandWrapper.getFirstChildElement().getStyle()
-                            .setMarginTop(-totalSize, Unit.PX);
-                } else {
-                    expandWrapper.getStyle().setPaddingLeft(totalSize, Unit.PX);
-                    expandWrapper.getFirstChildElement().getStyle()
-                            .setMarginLeft(-totalSize, Unit.PX);
-                }
-                recalculateExpands();
-
+                // TODO fails in Opera, always returns 0
+                totalSize += slot.getSpacingSize(vertical);
+            }
+
+            // When we set the margin to the first child, we don't need
+            // overflow:hidden in the layout root element, since the wrapper
+            // would otherwise be placed outside of the layout root element
+            // and block events on elements below it.
+            if (vertical) {
+                expandWrapper.getStyle().setPaddingTop(totalSize, Unit.PX);
+                expandWrapper.getFirstChildElement().getStyle()
+                        .setMarginTop(-totalSize, Unit.PX);
+            } else {
+                expandWrapper.getStyle().setPaddingLeft(totalSize, Unit.PX);
+                expandWrapper.getFirstChildElement().getStyle()
+                        .setMarginLeft(-totalSize, Unit.PX);
             }
+
+            recalculateExpands();
         }
     }
 
diff --git a/tests/testbench/com/vaadin/tests/components/table/TextFieldRelativeWidth.java b/tests/testbench/com/vaadin/tests/components/table/TextFieldRelativeWidth.java
index 865a50c5f7..ecf8d83ffa 100644
--- a/tests/testbench/com/vaadin/tests/components/table/TextFieldRelativeWidth.java
+++ b/tests/testbench/com/vaadin/tests/components/table/TextFieldRelativeWidth.java
@@ -27,8 +27,7 @@ public class TextFieldRelativeWidth extends TestBase {
 
     public class EditTable extends Table implements Button.ClickListener {
 
-        private Button addButton = new Button("Add new row",
-                (Button.ClickListener) this);
+        private Button addButton = new Button("Add new row", this);
 
         private String inputPrompt;
 
@@ -44,6 +43,7 @@ public class TextFieldRelativeWidth extends TestBase {
             setPageLength(100);
             setHeight("100%");
             setSizeFull();
+            getLayout().setSizeFull();
             addContainerProperty("id", Integer.class, null);
             addContainerProperty("text", Component.class, null);
             addContainerProperty("button", Button.class, null);
diff --git a/tests/testbench/com/vaadin/tests/components/textfield/TextFieldInLayoutInTable.java b/tests/testbench/com/vaadin/tests/components/textfield/TextFieldInLayoutInTable.java
index 3e1a709243..8dc33a47a4 100644
--- a/tests/testbench/com/vaadin/tests/components/textfield/TextFieldInLayoutInTable.java
+++ b/tests/testbench/com/vaadin/tests/components/textfield/TextFieldInLayoutInTable.java
@@ -29,6 +29,7 @@ public class TextFieldInLayoutInTable extends Application.LegacyApplication {
 
         table.setSizeFull();
         mainWindow.addComponent(table);
+        mainWindow.getContent().setSizeFull();
     }
 
 }
diff --git a/tests/testbench/com/vaadin/tests/themes/LiferayThemeTest.java b/tests/testbench/com/vaadin/tests/themes/LiferayThemeTest.java
new file mode 100644
index 0000000000..fa15d88799
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/themes/LiferayThemeTest.java
@@ -0,0 +1,37 @@
+package com.vaadin.tests.themes;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.terminal.WrappedRequest;
+import com.vaadin.tests.components.AbstractTestRoot;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Panel;
+import com.vaadin.ui.themes.LiferayTheme;
+
+@Theme("liferay")
+public class LiferayThemeTest extends AbstractTestRoot {
+
+    @Override
+    protected void setup(WrappedRequest request) {
+        Panel p = new Panel("Panel");
+        addComponent(p);
+        p.addComponent(new Label("Panel content"));
+
+        p = new Panel("Light Panel");
+        p.addStyleName(LiferayTheme.PANEL_LIGHT);
+        addComponent(p);
+        p.addComponent(new Label("Panel content"));
+    }
+
+    @Override
+    protected String getTestDescription() {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
+}