From: Artur Signell Date: Sat, 16 Apr 2016 10:47:26 +0000 (+0300) Subject: Take body scrolling into account when deciding combobox popup position (#19162) X-Git-Tag: 7.7.0.alpha2~21 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=034b61de038532aa4f444e3a4740da75459f8dda;p=vaadin-framework.git Take body scrolling into account when deciding combobox popup position (#19162) For standard standalone Vaadin apps, the body scroll is not scrollable. For these this change should have no effect. In embedded cases and when running as portlet, the body is scrollable. For these cases, the popup is now correctly shown below the combo box when there is space. Change-Id: Ia183f4f2988e6593634f6e9735fd58abbd4a1da1 --- diff --git a/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java b/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java index 8a6b442fb8..9fb6d18ac5 100644 --- a/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java +++ b/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java @@ -28,6 +28,7 @@ import com.google.gwt.aria.client.Roles; import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.core.client.Scheduler; import com.google.gwt.core.client.Scheduler.ScheduledCommand; +import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.NativeEvent; import com.google.gwt.dom.client.Style; @@ -657,7 +658,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, debug("VFS.SP: setPosition(" + offsetWidth + ", " + offsetHeight + ")"); - int top = topPosition; + int top; int left = getPopupLeft(); // reset menu size and retrieve its "natural" size @@ -705,19 +706,31 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, getContainerElement().getStyle().setWidth(rootWidth, Unit.PX); } - final int vfsHeight = VFilterSelect.this.getOffsetHeight(); - final int spaceAvailableAbove = top - vfsHeight; - final int spaceAvailableBelow = Window.getClientHeight() - top; - if (spaceAvailableBelow < offsetHeight - && spaceAvailableBelow < spaceAvailableAbove) { + final int textInputHeight = VFilterSelect.this.getOffsetHeight(); + final int textInputTopOnPage = tb.getAbsoluteTop(); + final int viewportOffset = Document.get().getScrollTop(); + final int textInputTopInViewport = textInputTopOnPage + - viewportOffset; + final int textInputBottomInViewport = textInputTopInViewport + + textInputHeight; + + final int spaceAboveInViewport = textInputTopInViewport; + final int spaceBelowInViewport = Window.getClientHeight() + - textInputBottomInViewport; + + if (spaceBelowInViewport < offsetHeight + && spaceBelowInViewport < spaceAboveInViewport) { // popup on top of input instead - top -= offsetHeight + vfsHeight; - if (top < 0) { - offsetHeight += top; - top = 0; + if (offsetHeight > spaceAboveInViewport) { + // Shrink popup height to fit above + offsetHeight = spaceAboveInViewport; } + top = textInputTopOnPage - offsetHeight; } else { - offsetHeight = Math.min(offsetHeight, spaceAvailableBelow); + // Show below, position calculated in showSuggestions for some + // strange reason + top = topPosition; + offsetHeight = Math.min(offsetHeight, spaceBelowInViewport); } // fetch real width (mac FF bugs here due GWT popups overflow:auto ) diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrolls.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrolls.java new file mode 100644 index 0000000000..ed8212b231 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrolls.java @@ -0,0 +1,51 @@ +/* + * 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.combobox; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.Label; + +public class ComboBoxPopupWhenBodyScrolls extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + getPage().getStyles().add( + "body.v-generated-body { overflow: auto;height:auto;}"); + getPage() + .getStyles() + .add("body.v-generated-body .v-ui.v-scrollable{ overflow: visible;height:auto !important;}"); + ComboBox cb = new ComboBox(); + for (int i = 0; i < 10; i++) { + cb.addItem("Item " + i); + } + + Label spacer = new Label("foo"); + spacer.setHeight("2000px"); + addComponent(spacer); + addComponent(cb); + spacer = new Label("foo"); + spacer.setHeight("2000px"); + addComponent(spacer); + // Chrome requires document.scrollTop () + // Firefox + IE wants document.documentElement.scrollTop () + getPage() + .getJavaScript() + .execute( + "document.body.scrollTop=1800;document.documentElement.scrollTop=1800;"); + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrollsTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrollsTest.java new file mode 100644 index 0000000000..0b1568b9b1 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPopupWhenBodyScrollsTest.java @@ -0,0 +1,40 @@ +/* + * 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.combobox; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.WebElement; + +import com.vaadin.tests.tb3.MultiBrowserTest; +import com.vaadin.tests.tb3.newelements.ComboBoxElement; + +public class ComboBoxPopupWhenBodyScrollsTest extends MultiBrowserTest { + + @Test + public void popupBelow() { + openTestURL(); + ComboBoxElement combobox = $(ComboBoxElement.class).first(); + combobox.openPopup(); + WebElement popup = $(ComboBoxElement.class).first() + .getSuggestionPopup(); + + int comboboxTop = combobox.getLocation().getY(); + int popupTop = popup.getLocation().getY(); + Assert.assertTrue("Popup should be below combobox", + popupTop > comboboxTop); + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/tb3/newelements/ComboBoxElement.java b/uitest/src/test/java/com/vaadin/tests/tb3/newelements/ComboBoxElement.java index 6a0f164b13..4a84c2bbc1 100644 --- a/uitest/src/test/java/com/vaadin/tests/tb3/newelements/ComboBoxElement.java +++ b/uitest/src/test/java/com/vaadin/tests/tb3/newelements/ComboBoxElement.java @@ -10,6 +10,9 @@ import com.vaadin.testbench.elementsbase.ServerClass; public class ComboBoxElement extends com.vaadin.testbench.elements.ComboBoxElement { + private static org.openqa.selenium.By bySuggestionPopup = By + .vaadin("#popup"); + public WebElement getInputField() { return findElement(By.vaadin("#textbox")); } @@ -24,6 +27,10 @@ public class ComboBoxElement extends getInputField().clear(); } + public WebElement getSuggestionPopup() { + return findElement(bySuggestionPopup); + } + @Override public void sendKeys(CharSequence... keysToSend) { sendKeys(50, keysToSend);