diff options
author | Olli Tietäväinen <ollit@vaadin.com> | 2017-11-13 10:18:46 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-11-13 10:18:46 +0200 |
commit | 7bb07cc56dea861c468be6d39fb62bcdd740d394 (patch) | |
tree | 780c3c59485342b7e2d20c2543e749bdcfc7c36e | |
parent | 1051b3c326db84dad9242356fab251676618314b (diff) | |
download | vaadin-framework-7bb07cc56dea861c468be6d39fb62bcdd740d394.tar.gz vaadin-framework-7bb07cc56dea861c468be6d39fb62bcdd740d394.zip |
hand-picked fix to #5043 combobox suggestion popup on scroll (#10307)
* hand-picked fix to #5043 combobox suggestion popup on scroll
* cleanup
3 files changed, 112 insertions, 9 deletions
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 af58756af0..5bddf00434 100644 --- 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 9f1c4b9e03..9023d28307 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 @@ -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 28fd9f4206..6c8a4cc298 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 @@ -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); |