Skip to content

Commit

Permalink
hand-picked fix to #5043 combobox suggestion popup on scroll (#10307)
Browse files Browse the repository at this point in the history 
* hand-picked fix to #5043 combobox suggestion popup on scroll

* cleanup
  • Loading branch information
@OlliTietavainenVaadin
OlliTietavainenVaadin committed Nov 13, 2017
1 parent 1051b3c commit 7bb07cc
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 9 deletions.
79 changes: 71 additions & 8 deletions client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
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;
Expand Down Expand Up @@ -55,6 +57,7 @@
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;
Expand Down Expand Up @@ -238,12 +241,12 @@ protected native JavaScriptObject createMousewheelListenerFunction(
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);
});
}-*/;
Expand Down Expand Up @@ -336,8 +339,10 @@ public class SuggestionPopup extends VOverlay
private int popupOuterPadding = -1;

private int topPosition;
private int leftPosition;

private final MouseWheeler mouseWheeler = new MouseWheeler();
private boolean scrollPending = false;

/**
* Default constructor
Expand Down Expand Up @@ -425,12 +430,11 @@ public void execute() {
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);
Expand Down Expand Up @@ -477,6 +481,22 @@ public void execute() {
});
}

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?
*
Expand Down Expand Up @@ -677,6 +697,47 @@ public void onBrowserEvent(Event event) {
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
Expand Down Expand Up @@ -1297,7 +1358,8 @@ public void selectLastItem() {
int getItemOffsetHeight() {
List<MenuItem> items = getItems();
return items != null && items.size() > 0
? items.get(0).getOffsetHeight() : 0;
? items.get(0).getOffsetHeight()
: 0;
}

/*
Expand All @@ -1306,7 +1368,8 @@ int getItemOffsetHeight() {
int getItemOffsetWidth() {
List<MenuItem> items = getItems();
return items != null && items.size() > 0
? items.get(0).getOffsetWidth() : 0;
? items.get(0).getOffsetWidth()
: 0;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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 {
Expand Down Expand Up @@ -35,6 +36,10 @@ protected void setup(VaadinRequest request) {
HorizontalLayout hl = new HorizontalLayout(combobox, combobox2,
combobox3, combobox4);
addComponent(hl);

Label spacer = new Label();
spacer.setHeight("800px");
addComponent(spacer);
}

}
37 changes: 36 additions & 1 deletion uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
* Copyright 2000-2014 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
Expand All @@ -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 {

Expand All @@ -43,6 +49,35 @@ public void testNoScrollbarsReindeer() {
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);

Expand Down

0 comments on commit 7bb07cc

Please sign in to comment.