From e175a3dd2c3c9f7cdc5a113721489bb8a1174919 Mon Sep 17 00:00:00 2001 From: Anna Miroshnik Date: Tue, 7 Oct 2014 19:37:30 +0400 Subject: [PATCH] Fix: Empty space on page after expanded component (#12672) Full defect name: Empty space on page after expanded component - incorrect height calculation in Chrome Layout: [ Panel (auto x auto) [ Grid (auto x auto) ] AnyComponent (100% x 100%) Also sleep() was removed from tests BaseLayoutExpandTest and BaseAddReplaceMoveTest Change-Id: Ie8a14a58dd53a26a133ea99a7b809d92c1b33a1f --- .../AbstractOrderedLayoutConnector.java | 15 +++- ...mptySpaceOnPageAfterExpandedComponent.java | 82 +++++++++++++++++++ ...SpaceOnPageAfterExpandedComponentTest.java | 61 ++++++++++++++ .../layouttester/BaseAddReplaceMoveTest.java | 5 +- .../layouttester/BaseLayoutExpandTest.java | 10 +-- 5 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponent.java create mode 100644 uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponentTest.java diff --git a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java index 0c09ae49c6..aace349392 100644 --- a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java +++ b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java @@ -152,7 +152,20 @@ public abstract class AbstractOrderedLayoutConnector extends public void onElementResize(ElementResizeEvent e) { updateLayoutHeight(); if (needsExpand()) { - getWidget().updateExpandCompensation(); + /* + * updateLayoutHeight causes calling of + * getLayoutManager().setNeedsMeasure(this) which informs this + * LayoutManager that the size of a component might have + * changed. Then a new layout phase is scheduled. So + * updateExpandCompensation must be delayed until layout phase + * will be completed. #12672 + */ + Scheduler.get().scheduleFinally(new ScheduledCommand() { + @Override + public void execute() { + getWidget().updateExpandCompensation(); + } + }); } } }; diff --git a/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponent.java b/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponent.java new file mode 100644 index 0000000000..c873a7efe7 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponent.java @@ -0,0 +1,82 @@ +/* + * 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.layout; + +import com.vaadin.server.Page; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.GridLayout; +import com.vaadin.ui.Panel; +import com.vaadin.ui.TextArea; +import com.vaadin.ui.TextField; +import com.vaadin.ui.VerticalLayout; + +public class EmptySpaceOnPageAfterExpandedComponent extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + getLayout().setHeight("200px"); + + VerticalLayout container = new VerticalLayout(); + container.setStyleName("mystyle"); + container.setId("container"); + container.setSpacing(true); + container.setSizeFull(); + addComponent(container); + + Page.getCurrent().getStyles() + .add(".mystyle {border: 1px solid black;}"); + + GridLayout grid = new GridLayout(); + grid.setSpacing(true); + + TextField text1 = new TextField(); + text1.setCaption("Text1"); + text1.setRequired(true); + + grid.setColumns(1); + grid.setRows(1); + + grid.addComponent(text1); + + grid.setSizeUndefined(); + + Panel panel = new Panel(); + panel.setContent(grid); + + panel.setSizeUndefined(); + + container.addComponent(panel); + + TextArea expand = new TextArea(); + expand.setId("expandedElement"); + expand.setSizeFull(); + container.addComponent(expand); + + container.setExpandRatio(expand, 1); + } + + @Override + protected String getTestDescription() { + return "Height calculation should be correct in Chrome. There should not be any empty space after expanded component."; + } + + @Override + protected Integer getTicketNumber() { + return 12672; + } +} \ No newline at end of file diff --git a/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponentTest.java b/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponentTest.java new file mode 100644 index 0000000000..09d19034e6 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/layout/EmptySpaceOnPageAfterExpandedComponentTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2000-2013 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.layout; + +import org.junit.Test; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.ui.ExpectedCondition; + +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Test to make sure that there is no any empty space (in Google Chrome) on page + * after expanded component (#12672) + * + * Layout: + * + * [ Panel (auto x auto) [ Grid (auto x auto) ] + * + * AnyComponent (100% x 100%) + * + * ] + * + * @author Vaadin Ltd + */ +public class EmptySpaceOnPageAfterExpandedComponentTest extends + MultiBrowserTest { + + @Test + public void testNoEmptySpaceOnPageAfterExpandedComponent() { + openTestURL(); + + final WebElement expandedElement = vaadinElementById("expandedElement"); + final WebElement containerElement = vaadinElementById("container"); + + waitUntil(new ExpectedCondition() { + @Override + public Boolean apply(WebDriver input) { + int expandedElementBottom = expandedElement.getLocation() + .getY() + expandedElement.getSize().getHeight(); + int containerElementBottom = containerElement.getLocation() + .getY() + containerElement.getSize().getHeight(); + + return expandedElementBottom + 1 == containerElementBottom; + } + }); + } +} diff --git a/uitest/src/com/vaadin/tests/layouts/layouttester/BaseAddReplaceMoveTest.java b/uitest/src/com/vaadin/tests/layouts/layouttester/BaseAddReplaceMoveTest.java index 73dc39009d..786e47cef1 100644 --- a/uitest/src/com/vaadin/tests/layouts/layouttester/BaseAddReplaceMoveTest.java +++ b/uitest/src/com/vaadin/tests/layouts/layouttester/BaseAddReplaceMoveTest.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; import org.junit.Test; +import org.openqa.selenium.By; import com.vaadin.testbench.elements.ButtonElement; import com.vaadin.tests.tb3.MultiBrowserTest; @@ -27,7 +28,7 @@ public abstract class BaseAddReplaceMoveTest extends MultiBrowserTest { @Test public void LayoutAlignment() throws IOException, InterruptedException { openTestURL(); - sleep(500); + waitForElementPresent(By.className("v-table")); compareScreen("initial"); String[] states = { "add", "replace", "move", "remove" }; List buttons = $(ButtonElement.class).all(); @@ -35,7 +36,7 @@ public abstract class BaseAddReplaceMoveTest extends MultiBrowserTest { // go through all buttons click them and see result for (ButtonElement btn : buttons) { btn.click(); - sleep(500); + waitForElementPresent(By.className("v-table")); compareScreen(states[index]); index++; } diff --git a/uitest/src/com/vaadin/tests/layouts/layouttester/BaseLayoutExpandTest.java b/uitest/src/com/vaadin/tests/layouts/layouttester/BaseLayoutExpandTest.java index 08f5aed808..036b053fb5 100644 --- a/uitest/src/com/vaadin/tests/layouts/layouttester/BaseLayoutExpandTest.java +++ b/uitest/src/com/vaadin/tests/layouts/layouttester/BaseLayoutExpandTest.java @@ -19,21 +19,17 @@ import java.io.IOException; import java.util.List; import org.junit.Test; +import org.openqa.selenium.By; import com.vaadin.testbench.elements.ButtonElement; import com.vaadin.tests.tb3.MultiBrowserTest; -/** - * - * @since - * @author Vaadin Ltd - */ public abstract class BaseLayoutExpandTest extends MultiBrowserTest { @Test public void LayoutExpand() throws IOException, InterruptedException { openTestURL(); - sleep(500); + waitForElementPresent(By.className("v-table")); compareScreen("initial"); String[] states = { "expand_100_0", "expand_50_50", "expand_25_75" }; List buttons = $(ButtonElement.class).all(); @@ -41,7 +37,7 @@ public abstract class BaseLayoutExpandTest extends MultiBrowserTest { // go through all buttons click them and see result for (ButtonElement btn : buttons) { btn.click(); - sleep(500); + waitForElementPresent(By.className("v-table")); compareScreen(states[index]); index++; } -- 2.39.5