From c24119222373072e67db3496bb9ff2466c22d734 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 25 Aug 2017 13:49:00 +0300 Subject: [PATCH] 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 --- .../java/com/vaadin/client/ui/VComboBox.java | 69 ++++++++++++++++--- .../combobox/ComboboxPopupScrolling.java | 16 +++-- .../combobox/ComboboxPopupScrollingTest.java | 28 ++++++++ 3 files changed, 99 insertions(+), 14 deletions(-) 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 bb75385007..784f2b116d 100644 --- 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 073aea4c95..6178b6e5c3 100644 --- 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 combobox = new ComboBox<>("100px wide combobox"); combobox.setWidth("100px"); combobox.setItems("AMERICAN SAMOA", "ANTIGUA AND BARBUDA"); - ComboBox combobox2 = new ComboBox("250px wide combobox"); + ComboBox combobox2 = new ComboBox<>("250px wide combobox"); combobox2.setWidth("250px"); combobox2.setItems("AMERICAN SAMOA", "ANTIGUA AND BARBUDA"); - ComboBox combobox3 = new ComboBox("Undefined wide combobox"); + ComboBox 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 combobox4 = new ComboBox<>( + "Another 100px wide combobox"); combobox4.setWidth("100px"); List 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 8e15030231..3a94153eb8 100644 --- 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); -- 2.39.5