Move ComboBox popup with ComboBox on scroll (#9869)

Keep the ComboBox popup at the same position relative to the ComboBox
when the view is scrolled.

Fixes #5043

diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
index bb75385007a508cb704f24d41a7f1067c693084a..784f2b116d34d0cb313b4ca3fbe531fad0ef2382 100644 (file)
--- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java
+++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
@@ -24,6 +24,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
+import com.google.gwt.animation.client.AnimationScheduler;
 import com.google.gwt.aria.client.Roles;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.Scheduler.ScheduledCommand;
@@ -55,6 +56,7 @@ import com.google.gwt.i18n.client.HasDirection.Direction;
 import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Event;
+import com.google.gwt.user.client.Event.NativePreviewEvent;
 import com.google.gwt.user.client.Timer;
 import com.google.gwt.user.client.Window;
 import com.google.gwt.user.client.ui.Composite;
@@ -342,9 +344,12 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
         private int popupOuterPadding = -1;
 
         private int topPosition;
+        private int leftPosition;
 
         private final MouseWheeler mouseWheeler = new MouseWheeler();
 
+        private boolean scrollPending = false;
+
         /**
          * Default constructor
          */
@@ -410,15 +415,10 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             // Add TT anchor point
             getElement().setId("VAADIN_COMBOBOX_OPTIONLIST");
 
-            final int x = toInt32(WidgetUtil
-                    .getBoundingClientRect(VComboBox.this.getElement())
-                    .getLeft());
-
-            topPosition = toInt32(WidgetUtil
-                    .getBoundingClientRect(tb.getElement()).getBottom())
-                    + Window.getScrollTop();
+            leftPosition = getDesiredLeftPosition();
+            topPosition = getDesiredTopPosition();
 
-            setPopupPosition(x, topPosition);
+            setPopupPosition(leftPosition, topPosition);
 
             int nullOffset = getNullSelectionItemShouldBeVisible() ? 1 : 0;
             boolean firstPage = currentPage == 0;
@@ -452,6 +452,17 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             setPopupPositionAndShow(popup);
         }
 
+        private int getDesiredTopPosition() {
+            return toInt32(WidgetUtil.getBoundingClientRect(tb.getElement())
+                    .getBottom()) + Window.getScrollTop();
+        }
+
+        private int getDesiredLeftPosition() {
+            return toInt32(WidgetUtil
+                    .getBoundingClientRect(VComboBox.this.getElement())
+                    .getLeft());
+        }
+
         private native int toInt32(double val)
         /*-{
             return val | 0;
@@ -660,6 +671,44 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             handleMouseDownEvent(event);
         }
 
+        @Override
+        protected void onPreviewNativeEvent(NativePreviewEvent event) {
+            // Check all events outside the combobox to see if they scroll the
+            // page. We cannot use e.g. Window.addScrollListener() because the
+            // scrolled element can be at any level on the page.
+
+            // Normally this is only called when the popup is showing, but make
+            // sure we don't accidentally process all events when not showing.
+            if (!scrollPending && isShowing() && !DOM.isOrHasChild(
+                    SuggestionPopup.this.getElement(),
+                    Element.as(event.getNativeEvent().getEventTarget()))) {
+                if (getDesiredLeftPosition() != leftPosition
+                        || getDesiredTopPosition() != topPosition) {
+                    updatePopupPositionOnScroll();
+                }
+            }
+
+            super.onPreviewNativeEvent(event);
+        }
+
+        /**
+         * Make the popup follow the position of the ComboBox when the page is
+         * scrolled.
+         */
+        private void updatePopupPositionOnScroll() {
+            if (!scrollPending) {
+                AnimationScheduler.get().requestAnimationFrame(timestamp -> {
+                    if (isShowing()) {
+                        leftPosition = getDesiredLeftPosition();
+                        topPosition = getDesiredTopPosition();
+                        setPopupPosition(leftPosition, topPosition);
+                    }
+                    scrollPending = false;
+                });
+                scrollPending = true;
+            }
+        }
+
         /**
          * Should paging be enabled. If paging is enabled then only a certain
          * amount of items are visible at a time and a scrollbar or buttons are
@@ -2830,7 +2879,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
 
     /**
      * Gets the empty selection caption.
-     * 
+     *
      * @since 8.0.7
      * @return the empty selection caption
      */
@@ -2844,7 +2893,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
      *
      * @param emptySelectionCaption
      *            the empty selection caption
-     * 
+     *
      * @since 8.0.7
      */
     public void setEmptySelectionCaption(String emptySelectionCaption) {

diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
index 073aea4c95f9e6d1c96d804163a9dd297e7dcdc9..6178b6e5c39571f4934401ccccfe97a9ff086f38 100644 (file)
--- a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
@@ -3,27 +3,31 @@ package com.vaadin.tests.components.combobox;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.ComboBox;
 import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Label;
 
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class ComboboxPopupScrolling extends AbstractTestUIWithLog {
     @Override
     protected void setup(VaadinRequest request) {
-        ComboBox combobox = new ComboBox("100px wide combobox");
+        ComboBox<String> combobox = new ComboBox<>("100px wide combobox");
         combobox.setWidth("100px");
         combobox.setItems("AMERICAN SAMOA", "ANTIGUA AND BARBUDA");
 
-        ComboBox combobox2 = new ComboBox("250px wide combobox");
+        ComboBox<String> combobox2 = new ComboBox<>("250px wide combobox");
         combobox2.setWidth("250px");
         combobox2.setItems("AMERICAN SAMOA", "ANTIGUA AND BARBUDA");
 
-        ComboBox combobox3 = new ComboBox("Undefined wide combobox");
+        ComboBox<String> combobox3 = new ComboBox<>("Undefined wide combobox");
         combobox3.setWidth(null);
         combobox3.setItems("AMERICAN SAMOA", "ANTIGUA AND BARBUDA");
 
-        ComboBox combobox4 = new ComboBox("Another 100px wide combobox");
+        ComboBox<String> combobox4 = new ComboBox<>(
+                "Another 100px wide combobox");
         combobox4.setWidth("100px");
         List<String> items = new ArrayList<>();
         for (int i = 0; i < 10; i++) {
@@ -35,6 +39,10 @@ public class ComboboxPopupScrolling extends AbstractTestUIWithLog {
         HorizontalLayout hl = new HorizontalLayout(combobox, combobox2,
                 combobox3, combobox4);
         addComponent(hl);
+
+        Label spacer = new Label();
+        spacer.setHeight("800px");
+        addComponent(spacer);
     }
 
 }
\ No newline at end of file

diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
index 8e15030231bece09cec9ae046fd9db2c692e3fa8..3a94153eb89648dbde82a1db70425aa4c682a482 100644 (file)
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
@@ -15,11 +15,14 @@
  */
 package com.vaadin.tests.components.combobox;
 
+import org.junit.Assert;
 import org.junit.Test;
 import org.openqa.selenium.By;
+import org.openqa.selenium.Point;
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elements.UIElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
 public class ComboboxPopupScrollingTest extends MultiBrowserTest {
@@ -44,6 +47,31 @@ public class ComboboxPopupScrollingTest extends MultiBrowserTest {
         testNoScrollbars("reindeer");
     }
 
+    @Test
+    public void testComboBoxTracksScrolledPage() {
+        openTestURL("theme=valo");
+
+        ComboBoxElement cb = $(ComboBoxElement.class).last();
+        cb.openPopup();
+        WebElement popup = cb.getSuggestionPopup();
+        Point comboLocation = cb.getLocation();
+        Point popupLocation = popup.getLocation();
+
+        // scroll page
+        $(UIElement.class).first().scroll(100);
+
+        // make sure animation frame is handled
+        sleep(500);
+
+        Point newComboLocation = cb.getLocation();
+        Point newPopupLocation = popup.getLocation();
+        Assert.assertNotEquals("ComboBox didn't move on the page", 0,
+                newComboLocation.y - comboLocation.y);
+        Assert.assertEquals("Popup didn't move with the combo box",
+                newComboLocation.y - comboLocation.y,
+                newPopupLocation.y - popupLocation.y);
+    }
+
     private void testNoScrollbars(String theme) {
         openTestURL("theme=" + theme);