hand-picked fix to #5043 combobox suggestion popup on scroll (#10307)

* hand-picked fix to #5043 combobox suggestion popup on scroll

* cleanup

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 af58756af07b55f9b0d2b4bef2d070cd836f1eee..5bddf004343943dc5857e71c582556f924137c2b 100644 (file)
--- a/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
@@ -24,6 +24,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
+import com.google.gwt.animation.client.AnimationScheduler;
+import com.google.gwt.animation.client.AnimationScheduler.AnimationCallback;
 import com.google.gwt.aria.client.Roles;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.Scheduler;
@@ -55,6 +57,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;
@@ -238,12 +241,12 @@ public class VFilterSelect extends Composite
             return $entry(function(e) {
                 var deltaX = e.deltaX ? e.deltaX : -0.5*e.wheelDeltaX;
                 var deltaY = e.deltaY ? e.deltaY : -0.5*e.wheelDeltaY;
-
+        
                 // IE8 has only delta y
                 if (isNaN(deltaY)) {
                     deltaY = -0.5*e.wheelDelta;
                 }
-
+        
                 @com.vaadin.client.ui.VFilterSelect.JsniUtil::moveScrollFromEvent(*)(widget, deltaX, deltaY, e, e.deltaMode);
             });
         }-*/;
@@ -336,8 +339,10 @@ public class VFilterSelect extends Composite
         private int popupOuterPadding = -1;
 
         private int topPosition;
+        private int leftPosition;
 
         private final MouseWheeler mouseWheeler = new MouseWheeler();
+        private boolean scrollPending = false;
 
         /**
          * Default constructor
@@ -425,12 +430,11 @@ public class VFilterSelect extends Composite
                     getElement().setId("VAADIN_COMBOBOX_OPTIONLIST");
 
                     menu.setSuggestions(currentSuggestions);
-                    final int x = VFilterSelect.this.getAbsoluteLeft();
+                    leftPosition = getDesiredLeftPosition();
 
-                    topPosition = tb.getAbsoluteTop();
-                    topPosition += tb.getOffsetHeight();
+                    topPosition = getDesiredTopPosition();
 
-                    setPopupPosition(x, topPosition);
+                    setPopupPosition(leftPosition, topPosition);
 
                     int nullOffset = (nullSelectionAllowed
                             && "".equals(lastFilter) ? 1 : 0);
@@ -477,6 +481,22 @@ public class VFilterSelect extends Composite
             });
         }
 
+        private native int toInt32(double val)
+        /*-{
+            return val | 0;
+        }-*/;
+
+        private int getDesiredTopPosition() {
+            return toInt32(WidgetUtil.getBoundingClientRect(tb.getElement())
+                    .getBottom()) + Window.getScrollTop();
+        }
+
+        private int getDesiredLeftPosition() {
+            return toInt32(WidgetUtil
+                    .getBoundingClientRect(VFilterSelect.this.getElement())
+                    .getLeft());
+        }
+
         /**
          * Should the next page button be visible to the user?
          *
@@ -677,6 +697,47 @@ public class VFilterSelect extends Composite
             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(new AnimationCallback() {
+                            public void execute(double 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
@@ -1297,7 +1358,8 @@ public class VFilterSelect extends Composite
         int getItemOffsetHeight() {
             List<MenuItem> items = getItems();
             return items != null && items.size() > 0
-                    ? items.get(0).getOffsetHeight() : 0;
+                    ? items.get(0).getOffsetHeight()
+                    : 0;
         }
 
         /*
@@ -1306,7 +1368,8 @@ public class VFilterSelect extends Composite
         int getItemOffsetWidth() {
             List<MenuItem> items = getItems();
             return items != null && items.size() > 0
-                    ? items.get(0).getOffsetWidth() : 0;
+                    ? items.get(0).getOffsetWidth()
+                    : 0;
         }
 
         /**

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 9f1c4b9e03388cca4ccf6036291a68706bde75cd..9023d283075c4851c5a4e8e9a75fba02332d992b 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
@@ -5,6 +5,7 @@ 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;
 
 @Theme("valo")
 public class ComboboxPopupScrolling extends AbstractTestUIWithLog {
@@ -35,6 +36,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 28fd9f4206d98e45a0b93a751bf66338c6f7fe08..6c8a4cc2982dd419dab192e847520395aaa5ef82 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
@@ -1,4 +1,4 @@
-/*
+    /*
  * Copyright 2000-2014 Vaadin Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -15,11 +15,17 @@
  */
 package com.vaadin.tests.components.combobox;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
 import org.junit.Test;
 import org.openqa.selenium.By;
+import org.openqa.selenium.Point;
 import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.elements.UIElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.newelements.ComboBoxElement;
 
 public class ComboboxPopupScrollingTest extends MultiBrowserTest {
 
@@ -43,6 +49,35 @@ public class ComboboxPopupScrollingTest extends MultiBrowserTest {
         testNoScrollbars("reindeer");
     }
 
+    @Test
+    public void testComboBoxTracksScrolledPage() {
+        openTestURL("theme=valo");
+
+        ComboBoxElement cb = $(ComboBoxElement.class).first();
+        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
+        try {
+            sleep(500);
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+
+        Point newComboLocation = cb.getLocation();
+        Point newPopupLocation = popup.getLocation();
+        assertNotEquals("ComboBox didn't move on the page", 0,
+                newComboLocation.y - comboLocation.y);
+        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);