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 8a6b442fb8ad2b17c1de54982e2e41e68c0c5e9a..9fb6d18ac560371f19d1c634b8762f1ab05d34be 100644 (file)
--- 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 (file)
index 0000000..ed8212b
--- /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 (<body>)
+        // Firefox + IE wants document.documentElement.scrollTop (<html>)
+        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 (file)
index 0000000..0b1568b
--- /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 6a0f164b13caa9730ee4d3bc5a85834adcc8a21c..4a84c2bbc1acce01460fc29c0e7d8c72ca5a39a2 100644 (file)
--- 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);