From 7326935a82ac0ff8e5ec73c4863a47f4fb9ff682 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Mon, 21 Jul 2014 14:21:40 +0200 Subject: Export fetchRootConfig status so it can be read by TK (#14053) Offline apps need to know when server errors are 500 or 400 in order to switch appropriatelly to the offline mode. Also we need exported the fetchRootConfig method and a reliable way to get loaded apps. Related with change I29635982514071e63221a9771d6729da14273ad3 [1] see temporal workaround in TouchKitServlet [1] https://dev.vaadin.com/review/#/c/4037/ Change-Id: I339ca697d035508a67d1eb24480cd12c4b9c6c0e --- WebContent/VAADIN/vaadinBootstrap.js | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/WebContent/VAADIN/vaadinBootstrap.js b/WebContent/VAADIN/vaadinBootstrap.js index df46d8bc72..ced077138f 100644 --- a/WebContent/VAADIN/vaadinBootstrap.js +++ b/WebContent/VAADIN/vaadinBootstrap.js @@ -101,7 +101,7 @@ return value; }; - var fetchRootConfig = function() { + var fetchRootConfig = function(callback) { log('Fetching root config'); var url = getConfig('browserDetailsUrl'); if (!url) { @@ -141,6 +141,12 @@ r.open('POST', url, true); r.onreadystatechange = function (aEvt) { if (r.readyState == 4) { + // Save responseStatus so as Offline Applications know what happened + // when loading root configuration from server, and depending on the + // error status display an error message or the offline UI. + config.rootResponseStatus = r.status; + config.rootResponseText = r.responseText; + var text = r.responseText; if (r.status == 200){ log("Got root config response", text); @@ -166,6 +172,9 @@ appDiv.innerHTML = text; appDiv.style['overflow'] = 'auto'; } + + // Run the fetchRootConfig callback if present. + callback && callback(r); } }; // send parameters as POST data @@ -177,7 +186,10 @@ //Export public data var app = { - 'getConfig': getConfig + getConfig: getConfig, + // Used when the app was started in offline, so as it is possible + // to defer root configuration loading until network is available. + fetchRootConfig: fetchRootConfig }; apps[appId] = app; @@ -224,9 +236,17 @@ return app; }, clients: {}, + getAppIds: function() { + var ids = [ ]; + for (var id in apps) { + if (apps.hasOwnProperty(id)) { + ids.push(id); + } + } + return ids; + }, getApp: function(appId) { - var app = apps[appId]; - return app; + return apps[appId]; }, loadTheme: loadTheme, registerWidgetset: function(widgetset, callback) { -- cgit v1.2.3 From d3266dfad77b07a00d46fdd5f4ef017203768a21 Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Fri, 18 Jul 2014 16:31:34 +0300 Subject: Fallback to finding disconnected UI based on AtmosphereResource (#14251) Change-Id: Icdac51322a90c32c122a182bc692c4eff3d8285b --- .../vaadin/server/communication/PushHandler.java | 50 +++++++++++++++--- .../vaadin/tests/push/RefreshCloseConnection.java | 61 ++++++++++++++++++++++ .../tests/push/RefreshCloseConnectionTest.java | 44 ++++++++++++++++ 3 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/push/RefreshCloseConnection.java create mode 100644 uitest/src/com/vaadin/tests/push/RefreshCloseConnectionTest.java diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java index 5a7ae5b3e8..93f1434c94 100644 --- a/server/src/com/vaadin/server/communication/PushHandler.java +++ b/server/src/com/vaadin/server/communication/PushHandler.java @@ -18,6 +18,7 @@ package com.vaadin.server.communication; import java.io.IOException; import java.io.Reader; +import java.util.Collection; import java.util.logging.Level; import java.util.logging.Logger; @@ -48,7 +49,7 @@ import com.vaadin.ui.UI; /** * Establishes bidirectional ("push") communication channels - * + * * @author Vaadin Ltd * @since 7.1 */ @@ -194,7 +195,7 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { /** * Find the UI for the atmosphere resource, lock it and invoke the callback. - * + * * @param resource * the atmosphere resource for the current request * @param callback @@ -357,9 +358,31 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { // Sets UI.currentInstance ui = service.findUI(vaadinRequest); if (ui == null) { - getLogger().log(Level.SEVERE, - "Could not get UI. This should never happen"); - return; + /* + * UI not found, could be because FF has asynchronously closed + * the websocket connection and Atmosphere has already done + * cleanup of the request attributes. + * + * In that case, we still have a chance of finding the right UI + * by iterating through the UIs in the session looking for one + * using the same AtmosphereResource. + */ + ui = findUiUsingResource(resource, session.getUIs()); + + if (ui == null) { + getLogger() + .log(Level.SEVERE, + "Could not get UI. This should never happen," + + " except when reloading in Firefox -" + + " see http://dev.vaadin.com/ticket/14251."); + return; + } else { + getLogger() + .log(Level.INFO, + "No UI was found based on data in the request," + + " but a slower lookup based on the AtmosphereResource succeeded." + + " See http://dev.vaadin.com/ticket/14251 for more details."); + } } PushMode pushMode = ui.getPushConfiguration().getPushMode(); @@ -411,6 +434,19 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { } } + private static UI findUiUsingResource(AtmosphereResource resource, + Collection uIs) { + for (UI ui : uIs) { + PushConnection pushConnection = ui.getPushConnection(); + if (pushConnection instanceof AtmospherePushConnection) { + if (((AtmospherePushConnection) pushConnection).getResource() == resource) { + return ui; + } + } + } + return null; + } + /** * Sends a refresh message to the given atmosphere resource. Uses an * AtmosphereResource instead of an AtmospherePushConnection even though it @@ -419,10 +455,10 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { * two push connections which try to use the same UI. Using the * AtmosphereResource directly guarantees the message goes to the correct * recipient. - * + * * @param resource * The atmosphere resource to send refresh to - * + * */ private static void sendRefreshAndDisconnect(AtmosphereResource resource) throws IOException { diff --git a/uitest/src/com/vaadin/tests/push/RefreshCloseConnection.java b/uitest/src/com/vaadin/tests/push/RefreshCloseConnection.java new file mode 100644 index 0000000000..4d02c4e62e --- /dev/null +++ b/uitest/src/com/vaadin/tests/push/RefreshCloseConnection.java @@ -0,0 +1,61 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.push; + +import com.vaadin.annotations.PreserveOnRefresh; +import com.vaadin.annotations.Push; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; + +@Push +@PreserveOnRefresh +public class RefreshCloseConnection extends AbstractTestUIWithLog { + + @Override + protected void setup(VaadinRequest request) { + log("Init"); + } + + @Override + protected void refresh(VaadinRequest request) { + if (getPushConnection().isConnected()) { + log("Still connected"); + } + log("Refresh"); + new Thread() { + @Override + public void run() { + accessSynchronously(new Runnable() { + @Override + public void run() { + log("Push"); + } + }); + } + }.start(); + } + + @Override + protected String getTestDescription() { + return "A log row should get pushed after reloading the page"; + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(14251); + } + +} diff --git a/uitest/src/com/vaadin/tests/push/RefreshCloseConnectionTest.java b/uitest/src/com/vaadin/tests/push/RefreshCloseConnectionTest.java new file mode 100644 index 0000000000..c5c6064555 --- /dev/null +++ b/uitest/src/com/vaadin/tests/push/RefreshCloseConnectionTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.push; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.tests.tb3.MultiBrowserTest; +import com.vaadin.tests.tb3.WebsocketTest; + +public class RefreshCloseConnectionTest extends MultiBrowserTest { + @Test + public void testSessionRefresh() { + openTestURL(); + + Assert.assertEquals("1. Init", getLogRow(0)); + + openTestURL(); + + Assert.assertEquals("2. Refresh", getLogRow(1)); + Assert.assertEquals("3. Push", getLogRow(0)); + } + + @Override + public List getBrowsersToTest() { + return WebsocketTest.getWebsocketBrowsers(); + } +} -- cgit v1.2.3 From 4e9139fce2e0ce1ebc2b29b6842c940d90f7df31 Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Tue, 22 Jul 2014 14:24:44 +0300 Subject: Update readme to mention Git settings for Windows users. Change-Id: Ie67372d1ea77ad025fe3cb42cfe7a6ecc94ee3df --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4af40fcb48..45421f3fbd 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,9 @@ Cloning the project repositories The Vaadin repository can be cloned using 
git clone https://github.com/vaadin/vaadin.git
-or using your favorite Git tool +or using your favorite Git tool. + +If using Windows, you might want to add these Git settings: core.autocrlf=false and core.fileMode=false. Setting up Eclipse to Develop Vaadin 7 ========= -- cgit v1.2.3 From 81645ab3566b34d8826f6b0e89938f00c83c9f4f Mon Sep 17 00:00:00 2001 From: Dmitrii Rogozin Date: Wed, 16 Jul 2014 17:20:28 +0300 Subject: Fix background color of Notification in chameleon theme (#14246) Change-Id: Ie561a3ef95fcc15e357d1edb65b45f596683e7e4 --- .../VAADIN/themes/chameleon/components/notification/notification.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WebContent/VAADIN/themes/chameleon/components/notification/notification.scss b/WebContent/VAADIN/themes/chameleon/components/notification/notification.scss index 783e4bcc1f..85fbb3295f 100644 --- a/WebContent/VAADIN/themes/chameleon/components/notification/notification.scss +++ b/WebContent/VAADIN/themes/chameleon/components/notification/notification.scss @@ -8,6 +8,8 @@ div.#{$primaryStyleName} { -webkit-box-shadow: 0 2px 5px rgba(0,0,0,.7); -moz-box-shadow: 0 2px 5px rgba(0,0,0,.7); box-shadow: 0 2px 5px rgba(0,0,0,.7); + //IE8 does not support rgba, using just rgb + background:rgb(255,255,255) url(../img/grad-light-top.png) repeat-x; background:rgba(255,255,255,.90) url(../img/grad-light-top.png) repeat-x; } -- cgit v1.2.3 From be2a71110a75bc83a7281c11b60285079e8cbffa Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Tue, 22 Jul 2014 19:02:21 +0300 Subject: Parallelize ide.xml (#14276) With this patch, the theme-and-default-widgetset target finishes in 50 seconds, whereas it takes about 80 seconds without the patch. This happens at the cost of peak memory usage rising from ~750mb to ~850mb. Change-Id: I969e9b3b01907e24bb8d411884d060ca1b539bde --- build/ide.xml | 128 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 72 insertions(+), 56 deletions(-) diff --git a/build/ide.xml b/build/ide.xml index 0775a67505..fa8b85ee23 100755 --- a/build/ide.xml +++ b/build/ide.xml @@ -4,61 +4,77 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + @@ -74,18 +90,18 @@ - + - + - + @@ -125,7 +141,7 @@ - + -- cgit v1.2.3 From f743e042f03f2c72d2f71507e475e3d26109024a Mon Sep 17 00:00:00 2001 From: Anthony Guerreiro Date: Wed, 2 Jul 2014 11:58:34 +0300 Subject: Fix NativeButton clickEvent coordinates in IE11 (#14022) Two clicks were being triggered for IE11, the first with coordinates (0,0) and the second with the correct coordinates. Change-Id: I6f0feb520710b254eac6542f082a5012de2c5f85 --- client/src/com/vaadin/client/ui/VNativeButton.java | 4 +- .../components/nativebutton/NativeButtonClick.java | 89 ++++++++++++++++++++++ .../nativebutton/NativeButtonClickTest.java | 71 +++++++++++++++++ 3 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClick.java create mode 100644 uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClickTest.java diff --git a/client/src/com/vaadin/client/ui/VNativeButton.java b/client/src/com/vaadin/client/ui/VNativeButton.java index 93d8d958d6..8e0dd2bce1 100644 --- a/client/src/com/vaadin/client/ui/VNativeButton.java +++ b/client/src/com/vaadin/client/ui/VNativeButton.java @@ -104,7 +104,9 @@ public class VNativeButton extends Button implements ClickHandler { } clickPending = false; } else if (event.getTypeInt() == Event.ONFOCUS) { - if (BrowserInfo.get().isIE() && clickPending) { + if (BrowserInfo.get().isIE() + && BrowserInfo.get().getBrowserMajorVersion() < 11 + && clickPending) { /* * The focus event will mess up IE and IE will not trigger the * mouse up event (which in turn triggers the click event) until diff --git a/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClick.java b/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClick.java new file mode 100644 index 0000000000..b6c80aea0c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClick.java @@ -0,0 +1,89 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.nativebutton; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Label; +import com.vaadin.ui.NativeButton; + +/** + * UI used to validate click coordinates reported from clicks on NativeButton + * elements. + * + * @author Vaadin Ltd + */ +@SuppressWarnings("serial") +public class NativeButtonClick extends AbstractTestUI { + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server. + * VaadinRequest) + */ + @Override + protected void setup(VaadinRequest request) { + final Label label1 = new Label("0,0"); + final Label label2 = new Label("0,0"); + + Button button1 = new NativeButton("Button1", + new NativeButton.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + label1.setValue(event.getClientX() + "," + + event.getClientY()); + } + }); + Button button2 = new NativeButton("Button2", + new NativeButton.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + label2.setValue(event.getClientX() + "," + + event.getClientY()); + } + }); + + HorizontalLayout layout = new HorizontalLayout(); + layout.addComponents(button1, button2, label1, label2); + layout.setSpacing(true); + addComponent(layout); + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription() + */ + @Override + protected String getTestDescription() { + return "Validate click event coordinates not erroneously returned as x=0, y=0"; + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber() + */ + @Override + protected Integer getTicketNumber() { + return 14022; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClickTest.java b/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClickTest.java new file mode 100644 index 0000000000..cab2acefff --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/nativebutton/NativeButtonClickTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.nativebutton; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.LabelElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Test to see if coordinates returned by click event on NativeButtons look + * good. (see #14022) + * + * @author Vaadin Ltd + */ +public class NativeButtonClickTest extends MultiBrowserTest { + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.tb3.MultiBrowserTest#getBrowsersToTest() + */ + + @Test + public void testClickCoordinates() { + openTestURL(); + + clickFirstButton(); + String eventCoordinates = getFirstLabelValue(); + Assert.assertNotEquals("0,0", eventCoordinates); + + clickSecondButton(); + eventCoordinates = getSecondLabelValue(); + Assert.assertNotEquals("0,0", eventCoordinates); + } + + private void clickFirstButton() { + ButtonElement button = $(ButtonElement.class).first(); + button.click(); + } + + private void clickSecondButton() { + ButtonElement button = $(ButtonElement.class).get(1); + button.click(); + } + + private String getFirstLabelValue() { + LabelElement label = $(LabelElement.class).get(1); + return label.getText(); + } + + private String getSecondLabelValue() { + LabelElement label = $(LabelElement.class).get(2); + return label.getText(); + } +} -- cgit v1.2.3 From a84a2a6d27c35e4b176dc9b3433a824263ea14d0 Mon Sep 17 00:00:00 2001 From: Juuso Valli Date: Thu, 17 Jul 2014 13:24:50 +0300 Subject: Alter TooltipInWindowTest to inherit from TooltipTest (#14240) Change-Id: I27c0a236d4dd654c1cf8d567752af9d1ea3c1de5 --- .../tests/components/window/TooltipInWindow.java | 60 -------------- .../components/window/TooltipInWindowTest.java | 95 ---------------------- .../com/vaadin/tests/tooltip/TooltipInWindow.java | 57 +++++++++++++ .../vaadin/tests/tooltip/TooltipInWindowTest.java | 66 +++++++++++++++ 4 files changed, 123 insertions(+), 155 deletions(-) delete mode 100644 uitest/src/com/vaadin/tests/components/window/TooltipInWindow.java delete mode 100644 uitest/src/com/vaadin/tests/components/window/TooltipInWindowTest.java create mode 100644 uitest/src/com/vaadin/tests/tooltip/TooltipInWindow.java create mode 100644 uitest/src/com/vaadin/tests/tooltip/TooltipInWindowTest.java diff --git a/uitest/src/com/vaadin/tests/components/window/TooltipInWindow.java b/uitest/src/com/vaadin/tests/components/window/TooltipInWindow.java deleted file mode 100644 index cd2cc7d060..0000000000 --- a/uitest/src/com/vaadin/tests/components/window/TooltipInWindow.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2000-2014 Vaadin Ltd. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ - -package com.vaadin.tests.components.window; - -import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; -import com.vaadin.ui.TextField; -import com.vaadin.ui.VerticalLayout; -import com.vaadin.ui.Window; - -public class TooltipInWindow extends AbstractTestUI { - - @Override - protected void setup(VaadinRequest request) { - VerticalLayout layout = new VerticalLayout(); - layout.setMargin(true); - Window window = new Window("Window", layout); - layout.setSizeUndefined(); - window.center(); - layout.addComponent(createTextField("tf1")); - - addWindow(window); - addComponent(createTextField("tf2")); - } - - private TextField createTextField(String id) { - TextField tf = new TextField("TextField with a tooltip"); - tf.setDescription("My tooltip"); - tf.setId(id); - getTooltipConfiguration().setOpenDelay(0); - getTooltipConfiguration().setQuickOpenDelay(0); - getTooltipConfiguration().setCloseTimeout(1000); - return tf; - } - - @Override - protected String getTestDescription() { - return "Tooltips should also work in a Window (as well as in other overlays)"; - } - - @Override - protected Integer getTicketNumber() { - return Integer.valueOf(9172); - } - -} diff --git a/uitest/src/com/vaadin/tests/components/window/TooltipInWindowTest.java b/uitest/src/com/vaadin/tests/components/window/TooltipInWindowTest.java deleted file mode 100644 index 412fd3049d..0000000000 --- a/uitest/src/com/vaadin/tests/components/window/TooltipInWindowTest.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2000-2014 Vaadin Ltd. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ -package com.vaadin.tests.components.window; - -import static org.junit.Assert.assertEquals; - -import org.junit.Test; -import org.openqa.selenium.WebElement; -import org.openqa.selenium.interactions.HasInputDevices; -import org.openqa.selenium.interactions.Mouse; -import org.openqa.selenium.interactions.internal.Coordinates; -import org.openqa.selenium.internal.Locatable; - -import com.vaadin.testbench.By; -import com.vaadin.tests.tb3.MultiBrowserTest; - -/** - * - * @since - * @author Vaadin Ltd - */ -public class TooltipInWindowTest extends MultiBrowserTest { - - @Test - public void testTooltipsInSubWindow() throws InterruptedException { - openTestURL(); - - WebElement textfield = vaadinElementById("tf1"); - Coordinates textfieldCoordinates = ((Locatable) textfield) - .getCoordinates(); - - Mouse mouse = ((HasInputDevices) getDriver()).getMouse(); - - // Show tooltip - mouse.mouseMove(textfieldCoordinates, 10, 10); - - sleep(100); - ensureVisibleTooltipPositionedCorrectly(); - assertEquals("My tooltip", getTooltipElement().getText()); - - // Hide tooltip - mouse.mouseMove(textfieldCoordinates, -100, -100); - sleep(2000); - - ensureHiddenTooltipPositionedCorrectly(); - assertEquals("", getTooltipElement().getText()); - - // Show tooltip again - mouse.mouseMove(textfieldCoordinates, 10, 10); - - sleep(100); - ensureVisibleTooltipPositionedCorrectly(); - assertEquals("My tooltip", getTooltipElement().getText()); - - // Hide tooltip - mouse.mouseMove(textfieldCoordinates, -100, -100); - sleep(2000); - - ensureHiddenTooltipPositionedCorrectly(); - assertEquals("", getTooltipElement().getText()); - - } - - private WebElement getTooltipContainerElement() { - return getDriver().findElement(By.className("v-tooltip")); - } - - private void ensureVisibleTooltipPositionedCorrectly() { - WebElement textfield = vaadinElementById("tf1"); - int tooltipX = getTooltipContainerElement().getLocation().getX(); - int textfieldX = textfield.getLocation().getX(); - assertGreaterOrEqual("Tooltip should be positioned on the textfield (" - + tooltipX + " < " + textfieldX + ")", tooltipX, textfieldX); - } - - private void ensureHiddenTooltipPositionedCorrectly() { - int tooltipX = getTooltipContainerElement().getLocation().getX(); - assertLessThanOrEqual( - "Tooltip should be positioned outside of viewport (was at " - + tooltipX + ")", tooltipX, -1000); - } -} diff --git a/uitest/src/com/vaadin/tests/tooltip/TooltipInWindow.java b/uitest/src/com/vaadin/tests/tooltip/TooltipInWindow.java new file mode 100644 index 0000000000..690b65432a --- /dev/null +++ b/uitest/src/com/vaadin/tests/tooltip/TooltipInWindow.java @@ -0,0 +1,57 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.vaadin.tests.tooltip; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.TextField; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.Window; + +public class TooltipInWindow extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + VerticalLayout layout = new VerticalLayout(); + layout.setMargin(true); + Window window = new Window("Window", layout); + layout.setSizeUndefined(); + window.center(); + layout.addComponent(createTextField("tf1")); + + addWindow(window); + addComponent(createTextField("tf2")); + } + + private TextField createTextField(String id) { + TextField tf = new TextField("TextField with a tooltip"); + tf.setDescription("My tooltip"); + tf.setId(id); + return tf; + } + + @Override + protected String getTestDescription() { + return "Tooltips should also work in a Window (as well as in other overlays)"; + } + + @Override + protected Integer getTicketNumber() { + return 9172; + } + +} diff --git a/uitest/src/com/vaadin/tests/tooltip/TooltipInWindowTest.java b/uitest/src/com/vaadin/tests/tooltip/TooltipInWindowTest.java new file mode 100644 index 0000000000..1c50bf5486 --- /dev/null +++ b/uitest/src/com/vaadin/tests/tooltip/TooltipInWindowTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.tooltip; + +import org.junit.Test; +import org.openqa.selenium.WebElement; + +import com.vaadin.testbench.By; +import com.vaadin.tests.tb3.TooltipTest; + +/** + * Test if tooltips in subwindows behave correctly + * + * @author Vaadin Ltd + */ +public class TooltipInWindowTest extends TooltipTest { + + @Test + public void testTooltipsInSubWindow() throws Exception { + openTestURL(); + + WebElement textfield = vaadinElementById("tf1"); + + checkTooltip(textfield, "My tooltip"); + + ensureVisibleTooltipPositionedCorrectly(textfield); + + clearTooltip(); + + checkTooltip(textfield, "My tooltip"); + + clearTooltip(); + } + + private WebElement getTooltipContainerElement() { + return getDriver().findElement(By.className("v-tooltip")); + } + + private void ensureVisibleTooltipPositionedCorrectly(WebElement textfield) + throws InterruptedException { + int tooltipX = getTooltip().getLocation().getX(); + int textfieldX = textfield.getLocation().getX(); + assertGreaterOrEqual("Tooltip should be positioned on the textfield (" + + tooltipX + " < " + textfieldX + ")", tooltipX, textfieldX); + } + + private void ensureHiddenTooltipPositionedCorrectly() { + int tooltipX = getTooltipContainerElement().getLocation().getX(); + assertLessThanOrEqual( + "Tooltip should be positioned outside of viewport (was at " + + tooltipX + ")", tooltipX, -1000); + } +} -- cgit v1.2.3 From 9b19675dffec603bc7e8fe6d973ed4edafaff136 Mon Sep 17 00:00:00 2001 From: Bogdan Udrescu Date: Tue, 8 Jul 2014 09:58:45 +0300 Subject: Bottom component click scroll up the parent panel in a window (#12943) Due to old fix for (#11994) the v-scrollable div of the window would expand to 110% of its size then immediately back to the original size. The first action, expanding the v-scrollable to 110% would decrease the scrollTop value of our panel, while increasing its height. When the revert back action would set the v-scrollable to its own size, the panel's scrollTop would remain decreased, causing the scroll bar to move up, hiding the ~10% at the bottom. Fixed by calling Util.runWebkitOverflowAutoFix(); instead of changing the height. Change-Id: I79eafd1f9500c2e4c10dadbfc7100608c0732e04 --- .../html-tests/BottomComponentScrollsUp.html | 96 +++++++++++++++++++ .../com/vaadin/client/ApplicationConnection.java | 1 - client/src/com/vaadin/client/Util.java | 18 ++++ client/src/com/vaadin/client/ui/VScrollTable.java | 23 +---- client/src/com/vaadin/client/ui/VWindow.java | 90 ++++++------------ .../com/vaadin/client/ui/table/TableConnector.java | 10 +- .../window/BottomComponentScrollsUp.java | 103 +++++++++++++++++++++ .../window/BottomComponentScrollsUpTest.java | 71 ++++++++++++++ 8 files changed, 323 insertions(+), 89 deletions(-) create mode 100644 WebContent/html-tests/BottomComponentScrollsUp.html create mode 100644 uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUp.java create mode 100644 uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUpTest.java diff --git a/WebContent/html-tests/BottomComponentScrollsUp.html b/WebContent/html-tests/BottomComponentScrollsUp.html new file mode 100644 index 0000000000..a264b38ba8 --- /dev/null +++ b/WebContent/html-tests/BottomComponentScrollsUp.html @@ -0,0 +1,96 @@ + + + +Bottom component scroll when focus - test with plain html to see the default behaviour + + + + + + + + +
+ + + + + +
+ + + + \ No newline at end of file diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java index 6abcdac487..5fcb2070ec 100644 --- a/client/src/com/vaadin/client/ApplicationConnection.java +++ b/client/src/com/vaadin/client/ApplicationConnection.java @@ -66,7 +66,6 @@ import com.google.gwt.user.client.Window.ClosingHandler; import com.google.gwt.user.client.ui.HasWidgets; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ApplicationConfiguration.ErrorMessage; -import com.vaadin.client.ApplicationConnection.ApplicationStoppedEvent; import com.vaadin.client.ResourceLoader.ResourceLoadEvent; import com.vaadin.client.ResourceLoader.ResourceLoadListener; import com.vaadin.client.communication.HasJavaScriptConnectorHelper; diff --git a/client/src/com/vaadin/client/Util.java b/client/src/com/vaadin/client/Util.java index f175bbe714..f9243dafe9 100644 --- a/client/src/com/vaadin/client/Util.java +++ b/client/src/com/vaadin/client/Util.java @@ -468,6 +468,24 @@ public class Util { return detectedScrollbarSize; } + /** + * Defers the execution of {@link #runWebkitOverflowAutoFix(Element)} + * + * @since + * @param elem + * with overflow auto + */ + public static void runWebkitOverflowAutoFixDeferred(final Element elem) { + Scheduler.get().scheduleDeferred(new Command() { + + @Override + public void execute() { + Util.runWebkitOverflowAutoFix(elem); + } + }); + + } + /** * Run workaround for webkits overflow auto issue. * diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index d88f7426ef..59645aa6d3 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -2251,13 +2251,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, * Ensures the column alignments are correct at initial loading.
* (child components widths are correct) */ - Scheduler.get().scheduleDeferred(new Command() { - - @Override - public void execute() { - Util.runWebkitOverflowAutoFix(scrollBodyPanel.getElement()); - } - }); + Util.runWebkitOverflowAutoFixDeferred(scrollBodyPanel.getElement()); hadScrollBars = willHaveScrollbarz; } @@ -6720,13 +6714,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets, Util.notifyParentOfSizeChange(VScrollTable.this, rendering); } } - Scheduler.get().scheduleDeferred(new Command() { - @Override - public void execute() { - Util.runWebkitOverflowAutoFix(scrollBodyPanel.getElement()); - } - }); + Util.runWebkitOverflowAutoFixDeferred(scrollBodyPanel.getElement()); forceRealignColumnHeaders(); } @@ -6863,13 +6852,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, // We must run the fix as a deferred command to prevent it from // overwriting the scroll position with an outdated value, see // #7607. - Scheduler.get().scheduleDeferred(new Command() { - - @Override - public void execute() { - Util.runWebkitOverflowAutoFix(scrollBodyPanel.getElement()); - } - }); + Util.runWebkitOverflowAutoFixDeferred(scrollBodyPanel.getElement()); } triggerLazyColumnAdjustment(false); diff --git a/client/src/com/vaadin/client/ui/VWindow.java b/client/src/com/vaadin/client/ui/VWindow.java index 1cee727bc9..83a0001ad8 100644 --- a/client/src/com/vaadin/client/ui/VWindow.java +++ b/client/src/com/vaadin/client/ui/VWindow.java @@ -1,12 +1,12 @@ /* * Copyright 2000-2014 Vaadin Ltd. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -73,7 +73,7 @@ import com.vaadin.shared.ui.window.WindowRole; /** * "Sub window" component. - * + * * @author Vaadin Ltd */ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, @@ -295,7 +295,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Returns true if this window is the topmost VWindow - * + * * @return */ private boolean isActive() { @@ -437,7 +437,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * is prevented. *

* This message is not visible on the screen. - * + * * @param topMessage * String provided when the user navigates with Shift-Tab keys to * the top of the window @@ -452,7 +452,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * key is prevented. *

* This message is not visible on the screen. - * + * * @param bottomMessage * String provided when the user navigates with the Tab key to * the bottom of the window @@ -465,7 +465,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * Gets the message that is provided to users of assistive devices when the * user reaches the top of the window when leaving a window with the tab key * is prevented. - * + * * @return the top message */ public String getTabStopTopAssistiveText() { @@ -476,7 +476,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * Gets the message that is provided to users of assistive devices when the * user reaches the bottom of the window when leaving a window with the tab * key is prevented. - * + * * @return the bottom message */ public String getTabStopBottomAssistiveText() { @@ -554,41 +554,11 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /* * Shake up the DOM a bit to make the window shed unnecessary - * scrollbars and resize correctly afterwards. This resulting code - * took over a week to summon forth, and involved some pretty hairy - * black magic. Don't touch it unless you know what you're doing! - * Fixes ticket #11994 + * scrollbars and resize correctly afterwards. The version fixing + * ticket #11994 which was changing the size to 110% was replaced + * with this due to ticket #12943 */ - Scheduler.get().scheduleFinally(new ScheduledCommand() { - @Override - public void execute() { - final com.google.gwt.dom.client.Element scrollable = contents - .getFirstChildElement(); - - // Adjusting the width or height may change the scroll - // position, so store the current position - int horizontalScrollPosition = scrollable.getScrollLeft(); - int verticalScrollPosition = scrollable.getScrollTop(); - - final String oldWidth = scrollable.getStyle().getWidth(); - final String oldHeight = scrollable.getStyle().getHeight(); - - scrollable.getStyle().setWidth(110, Unit.PCT); - scrollable.getOffsetWidth(); - scrollable.getStyle().setProperty("width", oldWidth); - - scrollable.getStyle().setHeight(110, Unit.PCT); - scrollable.getOffsetHeight(); - scrollable.getStyle().setProperty("height", oldHeight); - - // Restore the scroll position - scrollable.setScrollLeft(horizontalScrollPosition); - scrollable.setScrollTop(verticalScrollPosition); - - updateContentsSize(); - positionOrSizeUpdated(); - } - }); + Util.runWebkitOverflowAutoFix(contents.getFirstChildElement()); } } @@ -616,7 +586,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Sets the closable state of the window. Additionally hides/shows the close * button according to the new state. - * + * * @param closable * true if the window can be closed by the user */ @@ -638,7 +608,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * Returns the closable state of the sub window. If the sub window is * closable a decoration (typically an X) is shown to the user. By clicking * on the X the user can close the window. - * + * * @return true if the sub window is closable */ protected boolean isClosable() { @@ -902,7 +872,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Setter for the text for assistive devices the window caption is prefixed * with. - * + * * @param assistivePrefix * the assistivePrefix to set */ @@ -913,7 +883,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Getter for the text for assistive devices the window caption is prefixed * with. - * + * * @return the assistivePrefix */ public String getAssistivePrefix() { @@ -923,7 +893,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Setter for the text for assistive devices the window caption is postfixed * with. - * + * * @param assistivePostfix * the assistivePostfix to set */ @@ -934,7 +904,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Getter for the text for assistive devices the window caption is postfixed * with. - * + * * @return the assistivePostfix */ public String getAssistivePostfix() { @@ -1086,14 +1056,14 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * TODO check if we need to support this with touch based devices. - * + * * Checks if the cursor was inside the browser content area when the event * happened. - * + * * @param event * The event to be checked * @return true, if the cursor is inside the browser content area - * + * * false, otherwise */ private boolean cursorInsideBrowserContentArea(Event event) { @@ -1382,7 +1352,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * assistive devices when it is opened. *

* When the provided array is empty, an existing description is removed. - * + * * @param connectors * with the connectors of the widgets to use as description */ @@ -1420,7 +1390,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * Gets the connectors that are used as assistive description. Text * contained in these connectors will be read by assistive devices when the * window is opened. - * + * * @return list of previously set connectors */ public List getAssistiveDescription() { @@ -1429,14 +1399,14 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Sets the WAI-ARIA role the window. - * + * * This role defines how an assistive device handles a window. Available * roles are alertdialog and dialog (@see Roles * Model). - * + * * The default role is dialog. - * + * * @param role * WAI-ARIA role to set for the window */ @@ -1455,7 +1425,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * The value of the parameter doTabStop is stored and used for non-modal * windows. For modal windows, the handlers are always registered, while * preserving the stored value. - * + * * @param doTabStop * true to prevent leaving the window, false to allow leaving the * window for non modal windows @@ -1472,9 +1442,9 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /** * Adds a Handler for when user moves the window. - * + * * @since 7.1.9 - * + * * @return {@link HandlerRegistration} used to remove the handler */ public HandlerRegistration addMoveHandler(WindowMoveHandler handler) { diff --git a/client/src/com/vaadin/client/ui/table/TableConnector.java b/client/src/com/vaadin/client/ui/table/TableConnector.java index 017d1d1024..d37fd36522 100644 --- a/client/src/com/vaadin/client/ui/table/TableConnector.java +++ b/client/src/com/vaadin/client/ui/table/TableConnector.java @@ -21,7 +21,6 @@ import com.google.gwt.core.client.Scheduler; import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.Style.Position; -import com.google.gwt.user.client.Command; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ApplicationConnection; import com.vaadin.client.BrowserInfo; @@ -210,13 +209,8 @@ public class TableConnector extends AbstractHasComponentsConnector implements // by changing overflows as the length of the contents // *shouldn't* have changed (unless the number of rows // or the height of the widget has also changed) - Scheduler.get().scheduleDeferred(new Command() { - @Override - public void execute() { - Util.runWebkitOverflowAutoFix(getWidget().scrollBodyPanel - .getElement()); - } - }); + Util.runWebkitOverflowAutoFixDeferred(getWidget().scrollBodyPanel + .getElement()); } } else { getWidget().initializeRows(uidl, rowData); diff --git a/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUp.java b/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUp.java new file mode 100644 index 0000000000..2c5e415408 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUp.java @@ -0,0 +1,103 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.window; + +import java.util.ArrayList; +import java.util.List; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Alignment; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Panel; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.Window; + +/** + * Reproducing bug #12943 where an action on a Button or ComboBox placed at the + * bottom of a window in a scroll panel, will scroll up the parent panel. + * + * This was due to the fact that with the state confirmation notification from + * the server, the window.setVisible would be call again, and the hack that + * solved the scrollbars in a window (#11994) would cause the our bug. + * + * @since + * @author Vaadin Ltd + */ +@SuppressWarnings("serial") +public class BottomComponentScrollsUp extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + Button b = new Button("Open window"); + addComponent(b); + b.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + openWindow(); + } + + }); + + openWindow(); + } + + private void openWindow() { + Window w = new Window(); + w.setWidth("300px"); + w.setHeight("300px"); + w.center(); + + Panel p = createPanel(); + p.setSizeFull(); + + w.setContent(p); + + addWindow(w); + } + + private Panel createPanel() { + Panel p = new Panel(); + + VerticalLayout content = new VerticalLayout(); + p.setContent(content); + content.setHeight("500px"); + + List items = new ArrayList(); + items.add("1"); + items.add("2"); + items.add("3"); + + Button button = new Button("Press me"); + content.addComponent(button); + content.setComponentAlignment(button, Alignment.BOTTOM_CENTER); + return p; + } + + @Override + protected String getTestDescription() { + return "Interacting with a component at the bottom of scrollable panel within a subwindow scrolls up"; + } + + @Override + protected Integer getTicketNumber() { + return 12943; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUpTest.java b/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUpTest.java new file mode 100644 index 0000000000..3d0da2677b --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/window/BottomComponentScrollsUpTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.window; + +import java.io.IOException; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.Dimension; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; + +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Automatic test for fix for #12943. + * + * While testing without the fix, the test failed on both Chrome and PhantomJS. + * + * @since + * @author Vaadin Ltd + */ +public class BottomComponentScrollsUpTest extends MultiBrowserTest { + + @Override + public void setup() throws Exception { + super.setup(); + + openTestURL(); + } + + @Test + public void windowScrollTest() throws IOException, InterruptedException { + TestBenchElement panelScrollable = (TestBenchElement) getDriver() + .findElement(By.className("v-panel-content")); + Dimension panelScrollableSize = panelScrollable.getSize(); + + WebElement verticalLayout = panelScrollable.findElement(By + .className("v-verticallayout")); + Dimension verticalLayoutSize = verticalLayout.getSize(); + + panelScrollable.scroll(verticalLayoutSize.height); + + WebElement button = verticalLayout + .findElement(By.className("v-button")); + + button.click(); + + // Loose the focus from the button. + new Actions(getDriver()) + .moveToElement(panelScrollable, panelScrollableSize.width / 2, + panelScrollableSize.height / 2).click().build() + .perform(); + + compareScreen("window"); + } +} -- cgit v1.2.3 From 28702c006fe124988c03c99eea4e2d609407e47e Mon Sep 17 00:00:00 2001 From: Antti Tanhuanpää Date: Mon, 30 Jun 2014 17:07:50 +0300 Subject: Add scrollbars to ComboBox suggestion popup if low on screen estate (#11929) Change-Id: Idfeb20a385fc68c6527f1947bdbf238d9d4af918 --- client/src/com/vaadin/client/ui/VFilterSelect.java | 381 ++++++++++++++------- .../client/ui/combobox/ComboBoxConnector.java | 39 ++- .../src/com/vaadin/client/ui/menubar/MenuBar.java | 108 +++++- .../components/combobox/ComboBoxOnSmallScreen.java | 76 ++++ .../combobox/ComboBoxOnSmallScreenTest.java | 84 +++++ .../com/vaadin/tests/components/combobox/fi.png | Bin 0 -> 25094 bytes .../vaadin/tests/components/combobox/fi_small.png | Bin 0 -> 3576 bytes 7 files changed, 552 insertions(+), 136 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreen.java create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreenTest.java create mode 100644 uitest/src/com/vaadin/tests/components/combobox/fi.png create mode 100644 uitest/src/com/vaadin/tests/components/combobox/fi_small.png diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java index 7f67c39500..46f90c07fa 100644 --- a/client/src/com/vaadin/client/ui/VFilterSelect.java +++ b/client/src/com/vaadin/client/ui/VFilterSelect.java @@ -62,6 +62,7 @@ import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ApplicationConnection; import com.vaadin.client.BrowserInfo; import com.vaadin.client.ComponentConnector; +import com.vaadin.client.ComputedStyle; import com.vaadin.client.ConnectorMap; import com.vaadin.client.Focusable; import com.vaadin.client.UIDL; @@ -252,7 +253,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, /** * Shows the popup where the user can see the filtered options - * + * * @param currentSuggestions * The filtered suggestions * @param currentPage @@ -264,10 +265,8 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, final Collection currentSuggestions, final int currentPage, final int totalSuggestions) { - if (enableDebug) { - debug("VFS.SP: showSuggestions(" + currentSuggestions + ", " - + currentPage + ", " + totalSuggestions + ")"); - } + debug("VFS.SP: showSuggestions(" + currentSuggestions + ", " + + currentPage + ", " + totalSuggestions + ")"); /* * We need to defer the opening of the popup so that the parent DOM @@ -316,8 +315,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, status.setInnerText(""); } // We don't need to show arrows or statusbar if there is - // only one - // page + // only one page if (totalSuggestions <= pageLength || pageLength == 0) { setPagingEnabled(false); } else { @@ -346,7 +344,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, /** * Should the next page button be visible to the user? - * + * * @param active */ private void setNextButtonActive(boolean active) { @@ -366,7 +364,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, /** * Should the previous page button be visible to the user - * + * * @param active */ private void setPrevButtonActive(boolean active) { @@ -391,18 +389,13 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, */ public void selectNextItem() { debug("VFS.SP: selectNextItem()"); - final MenuItem cur = menu.getSelectedItem(); - final int index = 1 + menu.getItems().indexOf(cur); + + final int index = menu.getSelectedIndex() + 1; if (menu.getItems().size() > index) { - final MenuItem newSelectedItem = menu.getItems().get(index); - menu.selectItem(newSelectedItem); - tb.setText(newSelectedItem.getText()); - tb.setSelectionRange(lastFilter.length(), newSelectedItem - .getText().length() - lastFilter.length()); - - } else if (hasNextPage()) { - selectPopupItemWhenResponseIsReceived = Select.FIRST; - filterOptions(currentPage + 1, lastFilter); + selectItem(menu.getItems().get(index)); + + } else { + selectNextPage(); } } @@ -411,29 +404,61 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, */ public void selectPrevItem() { debug("VFS.SP: selectPrevItem()"); - final MenuItem cur = menu.getSelectedItem(); - final int index = -1 + menu.getItems().indexOf(cur); + + final int index = menu.getSelectedIndex() - 1; if (index > -1) { - final MenuItem newSelectedItem = menu.getItems().get(index); - menu.selectItem(newSelectedItem); - tb.setText(newSelectedItem.getText()); - tb.setSelectionRange(lastFilter.length(), newSelectedItem - .getText().length() - lastFilter.length()); + selectItem(menu.getItems().get(index)); + } else if (index == -1) { - if (currentPage > 0) { - selectPopupItemWhenResponseIsReceived = Select.LAST; - filterOptions(currentPage - 1, lastFilter); - } + selectPrevPage(); + } else { - final MenuItem newSelectedItem = menu.getItems().get( - menu.getItems().size() - 1); - menu.selectItem(newSelectedItem); - tb.setText(newSelectedItem.getText()); - tb.setSelectionRange(lastFilter.length(), newSelectedItem - .getText().length() - lastFilter.length()); + selectItem(menu.getItems().get(menu.getItems().size() - 1)); } } + /** + * Select the first item of the suggestions list popup. + * + * @since + */ + public void selectFirstItem() { + debug("VFS.SP: selectFirstItem()"); + selectItem(menu.getFirstItem()); + } + + /** + * Select the last item of the suggestions list popup. + * + * @since + */ + public void selectLastItem() { + debug("VFS.SP: selectLastItem()"); + selectItem(menu.getLastItem()); + } + + /* + * Sets the selected item in the popup menu. + */ + private void selectItem(final MenuItem newSelectedItem) { + menu.selectItem(newSelectedItem); + + String text = newSelectedItem != null ? newSelectedItem.getText() + : ""; + + // Set the icon. + FilterSelectSuggestion suggestion = (FilterSelectSuggestion) newSelectedItem + .getCommand(); + setSelectedItemIcon(suggestion.getIconUri()); + + // Set the text. + tb.setText(text); + tb.setSelectionRange(lastFilter.length(), text.length() + - lastFilter.length()); + + menu.updateKeyboardSelectedItem(); + } + /* * Using a timer to scroll up or down the pages so when we receive lots * of consecutive mouse wheel events the pages does not flicker. @@ -486,17 +511,10 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, } } - /* - * (non-Javadoc) - * - * @see - * com.google.gwt.user.client.ui.Widget#onBrowserEvent(com.google.gwt - * .user.client.Event) - */ - @Override public void onBrowserEvent(Event event) { debug("VFS.SP: onBrowserEvent()"); + if (event.getTypeInt() == Event.ONCLICK) { final Element target = DOM.eventGetTarget(event); if (target == up || target == DOM.getChild(up, 0)) { @@ -504,12 +522,24 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, } else if (target == down || target == DOM.getChild(down, 0)) { lazyPageScroller.scrollDown(); } + } else if (event.getTypeInt() == Event.ONMOUSEWHEEL) { - int velocity = event.getMouseWheelVelocityY(); - if (velocity > 0) { - lazyPageScroller.scrollDown(); - } else { - lazyPageScroller.scrollUp(); + + boolean scrollNotActive = !menu.isScrollActive(); + + debug("VFS.SP: onBrowserEvent() scrollNotActive: " + + scrollNotActive); + + if (scrollNotActive) { + int velocity = event.getMouseWheelVelocityY(); + + debug("VFS.SP: onBrowserEvent() velocity: " + velocity); + + if (velocity > 0) { + lazyPageScroller.scrollDown(); + } else { + lazyPageScroller.scrollUp(); + } } } @@ -525,7 +555,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, * amount of items are visible at a time and a scrollbar or buttons are * visible to change page. If paging is turned of then all options are * rendered into the popup menu. - * + * * @param paging * Should the paging be turned on? */ @@ -546,32 +576,29 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, isPagingEnabled = paging; } - /* - * (non-Javadoc) - * - * @see - * com.google.gwt.user.client.ui.PopupPanel$PositionCallback#setPosition - * (int, int) - */ - @Override public void setPosition(int offsetWidth, int offsetHeight) { - debug("VFS.SP: setPosition()"); + debug("VFS.SP: setPosition(" + offsetWidth + ", " + offsetHeight + + ")"); - int top = -1; - int left = -1; + int top = topPosition; + int left = getPopupLeft(); // reset menu size and retrieve its "natural" size menu.setHeight(""); - if (currentPage > 0) { + if (currentPage > 0 && !hasNextPage()) { // fix height to avoid height change when getting to last page menu.fixHeightTo(pageLength); } - offsetHeight = getOffsetHeight(); + final int desiredHeight = offsetHeight = getOffsetHeight(); final int desiredWidth = getMainWidth(); + + debug("VFS.SP: desired[" + desiredWidth + ", " + desiredHeight + + "]"); + Element menuFirstChild = menu.getElement().getFirstChildElement(); - int naturalMenuWidth = menuFirstChild.getOffsetWidth(); + final int naturalMenuWidth = menuFirstChild.getOffsetWidth(); if (popupOuterPadding == -1) { popupOuterPadding = Util.measureHorizontalPaddingAndBorder( @@ -581,7 +608,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, if (naturalMenuWidth < desiredWidth) { menu.setWidth((desiredWidth - popupOuterPadding) + "px"); menuFirstChild.getStyle().setWidth(100, Unit.PCT); - naturalMenuWidth = desiredWidth; } if (BrowserInfo.get().isIE()) { @@ -589,48 +615,72 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, * IE requires us to specify the width for the container * element. Otherwise it will be 100% wide */ - int rootWidth = naturalMenuWidth - popupOuterPadding; + int rootWidth = Math.max(desiredWidth, naturalMenuWidth) + - popupOuterPadding; getContainerElement().getStyle().setWidth(rootWidth, Unit.PX); } - if (offsetHeight + getPopupTop() > Window.getClientHeight() - + Window.getScrollTop()) { + final int vfsHeight = VFilterSelect.this.getOffsetHeight(); + final int spaceAvailableAbove = top - vfsHeight; + final int spaceAvailableBelow = Window.getClientHeight() - top; + if (spaceAvailableBelow < offsetHeight + && spaceAvailableBelow < spaceAvailableAbove) { // popup on top of input instead - top = getPopupTop() - offsetHeight - - VFilterSelect.this.getOffsetHeight(); + top -= offsetHeight + vfsHeight; if (top < 0) { + offsetHeight += top; top = 0; } } else { - top = getPopupTop(); - /* - * Take popup top margin into account. getPopupTop() returns the - * top value including the margin but the value we give must not - * include the margin. - */ - int topMargin = (top - topPosition); - top -= topMargin; + offsetHeight = Math.min(offsetHeight, spaceAvailableBelow); } // fetch real width (mac FF bugs here due GWT popups overflow:auto ) offsetWidth = menuFirstChild.getOffsetWidth(); - if (offsetWidth + getPopupLeft() > Window.getClientWidth() - + Window.getScrollLeft()) { + + if (offsetHeight < desiredHeight) { + int menuHeight = offsetHeight; + if (isPagingEnabled) { + menuHeight -= up.getOffsetHeight() + down.getOffsetHeight() + + status.getOffsetHeight(); + } else { + final ComputedStyle s = new ComputedStyle(menu.getElement()); + menuHeight -= s.getIntProperty("marginBottom") + + s.getIntProperty("marginTop"); + } + + // If the available page height is really tiny then this will be + // negative and an exception will be thrown on setHeight. + int menuElementHeight = menu.getItemOffsetHeight(); + if (menuHeight < menuElementHeight) { + menuHeight = menuElementHeight; + } + + menu.setHeight(menuHeight + "px"); + + final int naturalMenuWidthPlusScrollBar = naturalMenuWidth + + Util.getNativeScrollbarSize(); + if (offsetWidth < naturalMenuWidthPlusScrollBar) { + menu.setWidth(naturalMenuWidthPlusScrollBar + "px"); + } + } + + if (offsetWidth + left > Window.getClientWidth()) { left = VFilterSelect.this.getAbsoluteLeft() - + VFilterSelect.this.getOffsetWidth() - + Window.getScrollLeft() - offsetWidth; + + VFilterSelect.this.getOffsetWidth() - offsetWidth; if (left < 0) { left = 0; + menu.setWidth(Window.getClientWidth() + "px"); } - } else { - left = getPopupLeft(); } + setPopupPosition(left, top); + menu.scrollSelectionIntoView(); } /** * Was the popup just closed? - * + * * @return true if popup was just closed */ public boolean isJustClosed() { @@ -659,7 +709,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, /** * Updates style names in suggestion popup to help theme building. - * + * * @param uidl * UIDL for the whole combo box * @param componentState @@ -723,23 +773,34 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, super(true); debug("VFS.SM: constructor()"); addDomHandler(this, LoadEvent.getType()); + + setScrollEnabled(true); } /** * Fixes menus height to use same space as full page would use. Needed - * to avoid height changes when quickly "scrolling" to last page + * to avoid height changes when quickly "scrolling" to last page. + */ + public void fixHeightTo(int pageItemsCount) { + setHeight(getPreferredHeight(pageItemsCount)); + } + + /* + * Gets the preferred height of the menu including pageItemsCount items. */ - public void fixHeightTo(int pagelenth) { + String getPreferredHeight(int pageItemsCount) { if (currentSuggestions.size() > 0) { - final int pixels = pagelenth * (getOffsetHeight() - 2) - / currentSuggestions.size(); - setHeight((pixels + 2) + "px"); + final int pixels = (getPreferredHeight() / currentSuggestions + .size()) * pageItemsCount; + return pixels + "px"; + } else { + return ""; } } /** * Sets the suggestions rendered in the menu - * + * * @param suggestions * The suggestions to be rendered in the menu */ @@ -789,6 +850,8 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, client.updateVariable(paintableId, "page", 0, false); client.updateVariable(paintableId, "selected", new String[] {}, immediate); + afterUpdateClientVariables(); + suggestionPopup.hide(); return; } @@ -837,6 +900,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, lastNewItemString = enteredItemValue; client.updateVariable(paintableId, "newitem", enteredItemValue, immediate); + afterUpdateClientVariables(); } } else if (item != null && !"".equals(lastFilter) @@ -909,26 +973,75 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, } - public void selectFirstItem() { - debug("VFS.SM: selectFirstItem()"); - MenuItem firstItem = getItems().get(0); - selectItem(firstItem); - } - private MenuItem getKeyboardSelectedItem() { return keyboardSelectedItem; } - public void setKeyboardSelectedItem(MenuItem firstItem) { - keyboardSelectedItem = firstItem; + public void setKeyboardSelectedItem(MenuItem menuItem) { + keyboardSelectedItem = menuItem; + } + + /** + * @deprecated use {@link SuggestionPopup#selectFirstItem()} instead. + */ + @Deprecated + public void selectFirstItem() { + debug("VFS.SM: selectFirstItem()"); + MenuItem firstItem = getItems().get(0); + selectItem(firstItem); } + /** + * @deprecated use {@link SuggestionPopup#selectLastItem()} instead. + */ + @Deprecated public void selectLastItem() { debug("VFS.SM: selectLastItem()"); List items = getItems(); MenuItem lastItem = items.get(items.size() - 1); selectItem(lastItem); } + + /* + * Sets the keyboard item as the current selected one. + */ + void updateKeyboardSelectedItem() { + setKeyboardSelectedItem(getSelectedItem()); + } + + /* + * Gets the height of one menu item. + */ + int getItemOffsetHeight() { + List items = getItems(); + return items != null && items.size() > 0 ? items.get(0) + .getOffsetHeight() : 0; + } + + /* + * Gets the width of one menu item. + */ + int getItemOffsetWidth() { + List items = getItems(); + return items != null && items.size() > 0 ? items.get(0) + .getOffsetWidth() : 0; + } + + /** + * Returns true if the scroll is active on the menu element or if the + * menu currently displays the last page with less items then the + * maximum visibility (in which case the scroll is not active, but the + * scroll is active for any other page in general). + */ + @Override + public boolean isScrollActive() { + String height = getElement().getStyle().getHeight(); + String preferredHeight = getPreferredHeight(pageLength); + + return !(height == null || height.length() == 0 || height + .equals(preferredHeight)); + } + } /** @@ -1273,10 +1386,9 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, * Whether to send the options request immediately */ private void filterOptions(int page, String filter, boolean immediate) { - if (enableDebug) { - debug("VFS: filterOptions(" + page + ", " + filter + ", " - + immediate + ")"); - } + debug("VFS: filterOptions(" + page + ", " + filter + ", " + immediate + + ")"); + if (filter.equals(lastFilter) && currentPage == page) { if (!suggestionPopup.isAttached()) { suggestionPopup.showSuggestions(currentSuggestions, @@ -1297,8 +1409,11 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, waitingForFilteringResponse = true; client.updateVariable(paintableId, "filter", filter, false); client.updateVariable(paintableId, "page", page, immediate); + afterUpdateClientVariables(); + lastFilter = filter; currentPage = page; + } /** For internal use only. May be removed or replaced in the future. */ @@ -1401,10 +1516,13 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, setPromptingOff(text); } setSelectedItemIcon(suggestion.getIconUri()); + if (!(newKey.equals(selectedOptionKey) || ("".equals(newKey) && selectedOptionKey == null))) { selectedOptionKey = newKey; client.updateVariable(paintableId, "selected", new String[] { selectedOptionKey }, immediate); + afterUpdateClientVariables(); + // currentPage = -1; // forget the page } suggestionPopup.hide(); @@ -1597,28 +1715,22 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, switch (event.getNativeKeyCode()) { case KeyCodes.KEY_DOWN: suggestionPopup.selectNextItem(); - suggestionPopup.menu.setKeyboardSelectedItem(suggestionPopup.menu - .getSelectedItem()); + DOM.eventPreventDefault(DOM.eventGetCurrentEvent()); event.stopPropagation(); break; case KeyCodes.KEY_UP: suggestionPopup.selectPrevItem(); - suggestionPopup.menu.setKeyboardSelectedItem(suggestionPopup.menu - .getSelectedItem()); + DOM.eventPreventDefault(DOM.eventGetCurrentEvent()); event.stopPropagation(); break; case KeyCodes.KEY_PAGEDOWN: - if (hasNextPage()) { - filterOptions(currentPage + 1, lastFilter); - } + selectNextPage(); event.stopPropagation(); break; case KeyCodes.KEY_PAGEUP: - if (currentPage > 0) { - filterOptions(currentPage - 1, lastFilter); - } + selectPrevPage(); event.stopPropagation(); break; case KeyCodes.KEY_TAB: @@ -1664,6 +1776,26 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, } + /* + * Show the prev page. + */ + private void selectPrevPage() { + if (currentPage > 0) { + filterOptions(currentPage - 1, lastFilter); + selectPopupItemWhenResponseIsReceived = Select.LAST; + } + } + + /* + * Show the next page. + */ + private void selectNextPage() { + if (hasNextPage()) { + filterOptions(currentPage + 1, lastFilter); + selectPopupItemWhenResponseIsReceived = Select.FIRST; + } + } + /** * Triggered when a key was depressed * @@ -1707,15 +1839,21 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, if (currentSuggestion != null) { String text = currentSuggestion.getReplacementString(); setPromptingOff(text); + setSelectedItemIcon(currentSuggestion.getIconUri()); + selectedOptionKey = currentSuggestion.key; + } else { if (focused || readonly || !enabled) { setPromptingOff(""); } else { setPromptingOn(); } + setSelectedItemIcon(null); + selectedOptionKey = null; } + lastFilter = ""; suggestionPopup.hide(); } @@ -1837,6 +1975,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, if (client.hasEventListeners(this, EventId.FOCUS)) { client.updateVariable(paintableId, EventId.FOCUS, "", true); + afterUpdateClientVariables(); } ComponentConnector connector = ConnectorMap.get(client).getConnector( @@ -1913,6 +2052,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, if (client.hasEventListeners(this, EventId.BLUR)) { client.updateVariable(paintableId, EventId.BLUR, "", true); + afterUpdateClientVariables(); } } @@ -2091,4 +2231,15 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, com.google.gwt.user.client.Element captionElement) { AriaHelper.bindCaption(tb, captionElement); } + + /* + * Anything that should be set after the client updates the server. + */ + private void afterUpdateClientVariables() { + // We need this here to be consistent with the all the calls. + // Then set your specific selection type only after + // client.updateVariable() method call. + selectPopupItemWhenResponseIsReceived = Select.NONE; + } + } diff --git a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java index 78505d034c..8feb349a9e 100644 --- a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java +++ b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java @@ -28,7 +28,6 @@ import com.vaadin.client.ui.AbstractFieldConnector; import com.vaadin.client.ui.SimpleManagedLayout; import com.vaadin.client.ui.VFilterSelect; import com.vaadin.client.ui.VFilterSelect.FilterSelectSuggestion; -import com.vaadin.client.ui.menubar.MenuItem; import com.vaadin.shared.ui.Connect; import com.vaadin.shared.ui.combobox.ComboBoxConstants; import com.vaadin.shared.ui.combobox.ComboBoxState; @@ -157,7 +156,15 @@ public class ComboBoxConnector extends AbstractFieldConnector implements } // handle selection (null or a single value) - if (uidl.hasVariable("selected")) { + if (uidl.hasVariable("selected") + + // In case we're switching page no need to update the selection as the + // selection process didn't finish. + // && getWidget().selectPopupItemWhenResponseIsReceived == + // VFilterSelect.Select.NONE + // + ) { + String[] selectedKeys = uidl.getStringArrayVariable("selected"); if (selectedKeys.length > 0) { performSelection(selectedKeys[0]); @@ -169,12 +176,16 @@ public class ComboBoxConnector extends AbstractFieldConnector implements if ((getWidget().waitingForFilteringResponse && getWidget().lastFilter .toLowerCase().equals(uidl.getStringVariable("filter"))) || popupOpenAndCleared) { + getWidget().suggestionPopup.showSuggestions( getWidget().currentSuggestions, getWidget().currentPage, getWidget().totalMatches); + getWidget().waitingForFilteringResponse = false; + if (!getWidget().popupOpenerClicked && getWidget().selectPopupItemWhenResponseIsReceived != VFilterSelect.Select.NONE) { + // we're paging w/ arrows Scheduler.get().scheduleDeferred(new ScheduledCommand() { @Override @@ -222,26 +233,16 @@ public class ComboBoxConnector extends AbstractFieldConnector implements */ private void navigateItemAfterPageChange() { if (getWidget().selectPopupItemWhenResponseIsReceived == VFilterSelect.Select.LAST) { - getWidget().suggestionPopup.menu.selectLastItem(); + getWidget().suggestionPopup.selectLastItem(); } else { - getWidget().suggestionPopup.menu.selectFirstItem(); + getWidget().suggestionPopup.selectFirstItem(); } - // This is used for paging so we update the keyboard selection - // variable as well. - MenuItem activeMenuItem = getWidget().suggestionPopup.menu - .getSelectedItem(); - getWidget().suggestionPopup.menu - .setKeyboardSelectedItem(activeMenuItem); - - // Update text field to contain the correct text - getWidget().setTextboxText(activeMenuItem.getText()); - getWidget().tb.setSelectionRange( - getWidget().lastFilter.length(), - activeMenuItem.getText().length() - - getWidget().lastFilter.length()); - - getWidget().selectPopupItemWhenResponseIsReceived = VFilterSelect.Select.NONE; // reset + // If you're in between 2 requests both changing the page back and + // forth, you don't want this here, instead you need it before any + // other request. + // getWidget().selectPopupItemWhenResponseIsReceived = + // VFilterSelect.Select.NONE; // reset } private void performSelection(String selectedKey) { diff --git a/client/src/com/vaadin/client/ui/menubar/MenuBar.java b/client/src/com/vaadin/client/ui/menubar/MenuBar.java index b00665e766..726defafd5 100644 --- a/client/src/com/vaadin/client/ui/menubar/MenuBar.java +++ b/client/src/com/vaadin/client/ui/menubar/MenuBar.java @@ -38,6 +38,7 @@ import java.util.List; import com.google.gwt.core.client.Scheduler; import com.google.gwt.dom.client.Element; +import com.google.gwt.dom.client.Style.Overflow; import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Event; @@ -74,6 +75,9 @@ import com.vaadin.client.ui.VOverlay; public class MenuBar extends Widget implements PopupListener { private final Element body; + private final Element table; + private final Element outer; + private final ArrayList items = new ArrayList(); private MenuBar parentMenu; private PopupPanel popup; @@ -98,7 +102,7 @@ public class MenuBar extends Widget implements PopupListener { public MenuBar(boolean vertical) { super(); - final Element table = DOM.createTable(); + table = DOM.createTable(); body = DOM.createTBody(); DOM.appendChild(table, body); @@ -109,7 +113,7 @@ public class MenuBar extends Widget implements PopupListener { this.vertical = vertical; - final Element outer = DOM.createDiv(); + outer = DOM.createDiv(); DOM.appendChild(outer, table); setElement(outer); @@ -324,6 +328,37 @@ public class MenuBar extends Widget implements PopupListener { return selectedItem; } + /** + * Gets the first item from the menu or null if no items. + * + * @since + * @return the first item from the menu or null if no items. + */ + public MenuItem getFirstItem() { + return items != null && items.size() > 0 ? items.get(0) : null; + } + + /** + * Gest the last item from the menu or null if no items. + * + * @since + * @return the last item from the menu or null if no items. + */ + public MenuItem getLastItem() { + return items != null && items.size() > 0 ? items.get(items.size() - 1) + : null; + } + + /** + * Gets the index of the selected item. + * + * @since + * @return the index of the selected item. + */ + public int getSelectedIndex() { + return items != null ? items.indexOf(getSelectedItem()) : -1; + } + @Override protected void onDetach() { // When the menu is detached, make sure to close all of its children. @@ -468,6 +503,7 @@ public class MenuBar extends Widget implements PopupListener { public void selectItem(MenuItem item) { if (item == selectedItem) { + scrollItemIntoView(item); return; } @@ -480,6 +516,74 @@ public class MenuBar extends Widget implements PopupListener { } selectedItem = item; + + scrollItemIntoView(item); + } + + /* + * Scroll the specified item into view. + */ + private void scrollItemIntoView(MenuItem item) { + if (item != null) { + item.getElement().scrollIntoView(); + } + } + + /** + * Scroll the selected item into view. + * + * @since + */ + public void scrollSelectionIntoView() { + scrollItemIntoView(selectedItem); + } + + /** + * Sets the menu scroll enabled or disabled. + * + * @since + * @param enabled + * the enabled state of the scroll. + */ + public void setScrollEnabled(boolean enabled) { + if (enabled) { + if (vertical) { + outer.getStyle().setOverflowY(Overflow.AUTO); + } else { + outer.getStyle().setOverflowX(Overflow.AUTO); + } + + } else { + if (vertical) { + outer.getStyle().clearOverflowY(); + } else { + outer.getStyle().clearOverflowX(); + } + } + } + + /** + * Gets whether the scroll is activate for this menu. + * + * @since + * @return true if the scroll is active, otherwise false. + */ + public boolean isScrollActive() { + // Element element = getElement(); + // return element.getOffsetHeight() > DOM.getChild(element, 0) + // .getOffsetHeight(); + int outerHeight = outer.getOffsetHeight(); + int tableHeight = table.getOffsetHeight(); + return outerHeight < tableHeight; + } + + /** + * Gets the preferred height of the menu. + * + * @since + */ + protected int getPreferredHeight() { + return table.getOffsetHeight(); } /** diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreen.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreen.java new file mode 100644 index 0000000000..044214cecf --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreen.java @@ -0,0 +1,76 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.combobox; + +import com.vaadin.data.Item; +import com.vaadin.server.ClassResource; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.ComboBox; + +/** + * Test UI for issue #11929 where ComboBox suggestion popup hides the ComboBox + * itself obscuring the text input field. + * + * @author Vaadin Ltd + */ +public class ComboBoxOnSmallScreen extends AbstractTestUI { + + private static final String PID = "captionPID"; + + @Override + protected void setup(VaadinRequest request) { + addComponents(createComboBox()); + } + + @Override + protected String getTestDescription() { + return "Combobox hides what you are typing on small screen"; + } + + @Override + protected Integer getTicketNumber() { + return 11929; + } + + private ComboBox createComboBox() { + ComboBox cb = new ComboBox(); + cb.addContainerProperty(PID, String.class, ""); + cb.setItemCaptionPropertyId(PID); + + Object selectId = null; + + for (int i = 1; i < 22; ++i) { + final String v = "Item #" + i; + Object itemId = cb.addItem(); + + if (i == 9) { + selectId = itemId; + } + + Item item = cb.getItem(itemId); + item.getItemProperty(PID).setValue(v); + int flagIndex = i % 3; + cb.setItemIcon(itemId, new ClassResource( + flagIndex == 0 ? "fi_small.png" : flagIndex == 1 ? "fi.gif" + : "se.gif")); + } + + cb.select(selectId); + + return cb; + } +} diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreenTest.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreenTest.java new file mode 100644 index 0000000000..f48f2bbdeb --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxOnSmallScreenTest.java @@ -0,0 +1,84 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.combobox; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.Dimension; +import org.openqa.selenium.WebDriver.Window; +import org.openqa.selenium.WebElement; + +import com.vaadin.client.ui.VFilterSelect; +import com.vaadin.testbench.elements.ComboBoxElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * ComboBox suggestion popup should not obscure the text input box. + * + * @author Vaadin Ltd + */ +public class ComboBoxOnSmallScreenTest extends MultiBrowserTest { + + private static final Dimension TARGETSIZE = new Dimension(600, 300); + private static final String POPUPCLASSNAME = VFilterSelect.CLASSNAME + + "-suggestpopup"; + + ComboBoxElement combobox; + WebElement popup; + + @Override + public void setup() throws Exception { + super.setup(); + + openTestURL(); + + getWindow().setSize(TARGETSIZE); + + combobox = $(ComboBoxElement.class).first(); + combobox.openPopup(); + + popup = findElement(By.className(POPUPCLASSNAME)); + } + + @Test + public void testSuggestionPopupOverlayPosition() { + final int popupTop = popup.getLocation().y; + final int popupBottom = popupTop + popup.getSize().getHeight(); + final int cbTop = combobox.getLocation().y; + final int cbBottom = cbTop + combobox.getSize().getHeight(); + + assertThat("Popup overlay overlaps with the textbox", + popupTop >= cbBottom || popupBottom <= cbTop, is(true)); + } + + @Test + public void testSuggestionPopupOverlaySize() { + final int popupTop = popup.getLocation().y; + final int popupBottom = popupTop + popup.getSize().getHeight(); + final int rootHeight = findElement(By.tagName("body")).getSize().height; + + assertThat("Popup overlay out of the screen", popupTop < 0 + || popupBottom > rootHeight, is(false)); + } + + private Window getWindow() { + return getDriver().manage().window(); + } + +} diff --git a/uitest/src/com/vaadin/tests/components/combobox/fi.png b/uitest/src/com/vaadin/tests/components/combobox/fi.png new file mode 100644 index 0000000000..976a9663ce Binary files /dev/null and b/uitest/src/com/vaadin/tests/components/combobox/fi.png differ diff --git a/uitest/src/com/vaadin/tests/components/combobox/fi_small.png b/uitest/src/com/vaadin/tests/components/combobox/fi_small.png new file mode 100644 index 0000000000..2908973fa4 Binary files /dev/null and b/uitest/src/com/vaadin/tests/components/combobox/fi_small.png differ -- cgit v1.2.3 From 53a62dba548c52824066eb30b7ad122df6e270a4 Mon Sep 17 00:00:00 2001 From: Heikki Ohinmaa Date: Mon, 28 Jul 2014 11:59:11 +0300 Subject: ComboBoxIdenticalItems test rewrite to TB3 Change-Id: I6127826f4ffbb1358821fb7ca5eabf4d774d792c --- .../combobox/ComboBoxIdenticalItems.html | 116 --------------------- .../combobox/ComboBoxIdenticalItems.java | 17 ++- .../combobox/ComboBoxIdenticalItemsTest.java | 76 ++++++++++++++ 3 files changed, 84 insertions(+), 125 deletions(-) delete mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.html create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItemsTest.java diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.html b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.html deleted file mode 100644 index 3ad7d62a09..0000000000 --- a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.html +++ /dev/null @@ -1,116 +0,0 @@ - - - - - - -New Test - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
New Test
open/run/com.vaadin.tests.components.combobox.ComboBoxIdenticalItems?restartApplication
mouseClickvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]66,8
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]enter
assertTextvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::PID_SLog_row_01. Item one-1 selected
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]enter
assertTextvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::PID_SLog_row_02. Item one-2 selected
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]down
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]enter
assertTextvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::PID_SLog_row_03. Item two selected
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]up
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]up
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]up
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]enter
pause100
assertTextvaadin=runcomvaadintestscomponentscomboboxComboBoxIdenticalItems::PID_SLog_row_04. Item one-1 selected
- - diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.java index 5f33b96a73..cdae1c8e38 100644 --- a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.java +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItems.java @@ -3,16 +3,18 @@ package com.vaadin.tests.components.combobox; import com.vaadin.data.Item; import com.vaadin.data.Property; import com.vaadin.data.Property.ValueChangeEvent; -import com.vaadin.tests.components.TestBase; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; import com.vaadin.tests.util.Log; import com.vaadin.ui.ComboBox; -public class ComboBoxIdenticalItems extends TestBase { +public class ComboBoxIdenticalItems extends AbstractTestUI { private Log log = new Log(5); + @SuppressWarnings("unchecked") @Override - public void setup() { + protected void setup(VaadinRequest request) { final ComboBox select = new ComboBox("ComboBox"); select.addContainerProperty("caption", String.class, null); Item item = select.addItem("one-1"); @@ -24,9 +26,7 @@ public class ComboBoxIdenticalItems extends TestBase { select.setItemCaptionPropertyId("caption"); select.setNullSelectionAllowed(false); select.setImmediate(true); - select.addListener(new Property.ValueChangeListener() { - private static final long serialVersionUID = -7932700771673919620L; - + select.addValueChangeListener(new Property.ValueChangeListener() { @Override public void valueChange(ValueChangeEvent event) { log.log("Item " + select.getValue() + " selected"); @@ -39,7 +39,7 @@ public class ComboBoxIdenticalItems extends TestBase { } @Override - protected String getDescription() { + protected String getTestDescription() { return "Keyboard selecting of a value is broken in combobox if two " + "items have the same caption. The first item's id is \"One-1\" " + "while the second one is \"One-2\". Selecting with mouse works " @@ -49,7 +49,6 @@ public class ComboBoxIdenticalItems extends TestBase { @Override protected Integer getTicketNumber() { - // TODO Auto-generated method stub - return null; + return 6125; } } diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItemsTest.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItemsTest.java new file mode 100644 index 0000000000..4da49a798f --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxIdenticalItemsTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.combobox; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.Keys; +import org.openqa.selenium.WebElement; + +import com.vaadin.testbench.By; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * @author Vaadin Ltd + */ +public class ComboBoxIdenticalItemsTest extends MultiBrowserTest { + + private WebElement select; + + /* This test has been directly ported from a TB2 test */ + @Test + public void identicalItemsKeyboardTest() throws Exception { + openTestURL(); + + // wait for the UI to be fully loaded + waitForElementToBePresent(By.className("v-filterselect")); + waitForElementToBePresent(By.id("Log")); + + select = findElement(By + .vaadin("/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VFilterSelect[0]/domChild[0]")); + select.click(); + + Keys[] downDownEnter = new Keys[] { Keys.ARROW_DOWN, Keys.ARROW_DOWN, + Keys.ENTER }; + sendKeys(downDownEnter); + assertLogText("1. Item one-1 selected"); + + sendKeys(downDownEnter); + assertLogText("2. Item one-2 selected"); + + sendKeys(downDownEnter); + assertLogText("3. Item two selected"); + + sendKeys(new Keys[] { Keys.ARROW_UP, Keys.ARROW_UP, Keys.ARROW_UP, + Keys.ENTER }); + assertLogText("4. Item one-1 selected"); + } + + private void assertLogText(String expected) throws Exception { + String text = findElement(By.vaadin("PID_SLog_row_0")).getText(); + Assert.assertTrue("Expected '" + expected + "' found '" + text + "'", + text.equals(expected)); + } + + private void sendKeys(Keys[] keys) throws Exception { + for (Keys key : keys) { + select.sendKeys(key); + // wait a while between the key presses, at least PhantomJS fails if + // they are sent too fast + sleep(10); + } + } +} -- cgit v1.2.3 From 441371ac98b2a99822ca1639a6823b23be93c229 Mon Sep 17 00:00:00 2001 From: Markus Koivisto Date: Thu, 26 Jun 2014 16:35:42 +0300 Subject: Keyboard shift-selection now works as expected in VScrollTable. (#14094) Change-Id: I0dcd9f75cd30fe91c17ca0755241e73a37da79ec --- client/src/com/vaadin/client/ui/VScrollTable.java | 19 ++++---- .../table/SelectAllConstantViewport.java | 6 ++- .../table/SelectAllConstantViewportTest.java | 57 +++++++++++++++++++++- .../src/com/vaadin/tests/tb3/MultiBrowserTest.java | 10 +++- 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 59645aa6d3..5e6207f53f 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -1093,9 +1093,17 @@ public class VScrollTable extends FlowPanel implements HasWidgets, /* * The focus is no longer on a selected row. Move * focus to the selected row. (#10522) + * + * Don't do this for multiselect (#13341). + * + * Checking the selection mode here instead of in + * setRowFocus allows keyboard shift+downarrow + * selection to work as expected. */ + if (isSingleSelectMode()) { + setRowFocus(row); + } - setRowFocus(row); } } if (selected != row.isSelected()) { @@ -7248,14 +7256,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, // Set new focused row focusedRow = row; - /* - * Don't scroll to the focused row when in multiselect mode. - * (#13341) - */ - - if (isSingleSelectMode()) { - ensureRowIsVisible(row); - } + ensureRowIsVisible(row); return true; } diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewport.java b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewport.java index 5a406eac48..35ac63efe2 100644 --- a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewport.java +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewport.java @@ -65,7 +65,7 @@ public class SelectAllConstantViewport extends AbstractTestUIWithLog { table.addItem(new Object[] { new Integer(i) }, new Integer(i)); } - table.setCurrentPageFirstItemIndex(185); + table.setCurrentPageFirstItemIndex(178); final CssLayout layout = new CssLayout(); layout.addComponent(selectAllCheckbox); @@ -82,7 +82,9 @@ public class SelectAllConstantViewport extends AbstractTestUIWithLog { @Override protected String getTestDescription() { - return "The scroll position of a table with many items should remain constant if all items are selected."; + return "The scroll position of a table with many items should remain constant if all items are " + + "selected. The scroll position should change if the user uses the keyboard to select " + + "multiple lines with shift+arrowkeys."; } /* diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java index 0e7a7c08a4..257efef6a2 100644 --- a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java @@ -15,14 +15,20 @@ */ package com.vaadin.tests.components.table; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import java.io.IOException; +import java.util.List; import org.junit.Test; +import org.openqa.selenium.Keys; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; +import org.openqa.selenium.remote.DesiredCapabilities; import org.openqa.selenium.support.ui.ExpectedCondition; import com.vaadin.testbench.By; @@ -33,7 +39,7 @@ import com.vaadin.tests.tb3.MultiBrowserTest; public class SelectAllConstantViewportTest extends MultiBrowserTest { @Test - public void testViewportUnchanged() throws IOException { + public void testViewportUnchangedwithMultiSel() throws IOException { openTestURL(); CheckBoxElement checkbox = $(CheckBoxElement.class).first(); @@ -51,11 +57,58 @@ public class SelectAllConstantViewportTest extends MultiBrowserTest { int rowLocation = row.getLocation().getY(); - // use click x,y with non-zero offset to actually toggle the checkbox. (#13763) + // use click x,y with non-zero offset to actually toggle the checkbox. + // (#13763) checkbox.click(5, 5); int newRowLocation = row.getLocation().getY(); assertThat(newRowLocation, is(rowLocation)); } + + @Test + public void testViewportChangedwithKeyboardSel() throws IOException { + openTestURL(); + + WebElement cell = $(TableElement.class).first().getCell(190, 0); + final WebElement scrollPositionDisplay = getDriver().findElement( + By.className("v-table-scrollposition")); + waitUntilNot(new ExpectedCondition() { + + @Override + public Boolean apply(WebDriver input) { + return scrollPositionDisplay.isDisplayed(); + } + }, 10); + + int rowLocation = cell.getLocation().getY(); + + // select downwards with shift (#14094) + + cell.click(); + + final WebElement row = getDriver().findElement( + By.className("v-selected")); + + assertThat(row.getAttribute("class"), containsString("selected")); + + // for some reason phantomJS does not support keyboard actions + + Actions action = new Actions(getDriver()); + action.keyDown(Keys.SHIFT) + .sendKeys(Keys.ARROW_DOWN, Keys.ARROW_DOWN, Keys.ARROW_DOWN, + Keys.ARROW_DOWN, Keys.ARROW_DOWN, Keys.ARROW_DOWN, + Keys.ARROW_DOWN).keyUp(Keys.SHIFT).build().perform(); + + int newRowLocation = cell.getLocation().getY(); + + assertThat(newRowLocation, is(not(rowLocation))); + + } + + @Override + public List getBrowsersToTest() { + // phantomJS does not support keyboard actions + return getBrowsersExcludingPhantomJS(); + } } diff --git a/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java b/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java index ccbb6ca872..ffa0b83dc2 100644 --- a/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java +++ b/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java @@ -41,7 +41,8 @@ import org.openqa.selenium.remote.DesiredCapabilities; public abstract class MultiBrowserTest extends PrivateTB3Configuration { protected List getBrowsersExcludingIE() { - List browsers = new ArrayList(getAllBrowsers()); + List browsers = new ArrayList( + getAllBrowsers()); browsers.remove(Browser.IE8.getDesiredCapabilities()); browsers.remove(Browser.IE9.getDesiredCapabilities()); browsers.remove(Browser.IE10.getDesiredCapabilities()); @@ -50,6 +51,13 @@ public abstract class MultiBrowserTest extends PrivateTB3Configuration { return browsers; } + protected List getBrowsersExcludingPhantomJS() { + List browsers = new ArrayList( + getAllBrowsers()); + browsers.remove(Browser.PHANTOMJS.getDesiredCapabilities()); + return browsers; + } + public enum Browser { FIREFOX(BrowserUtil.firefox(24)), CHROME(BrowserUtil.chrome(33)), SAFARI( BrowserUtil.safari(7)), IE8(BrowserUtil.ie(8)), IE9(BrowserUtil -- cgit v1.2.3 From 0182043e6cdb7ced238fe97090cc337941b11779 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 25 Jul 2014 14:29:42 +0300 Subject: Reduce TestBench 3 socket timeout (#14298) Change-Id: I45907c5c22bcea1a403af71ecbf5236e5669ab38 --- uitest/src/com/vaadin/tests/tb3/TB3Runner.java | 37 +++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/uitest/src/com/vaadin/tests/tb3/TB3Runner.java b/uitest/src/com/vaadin/tests/tb3/TB3Runner.java index 5b5a6dcf39..4241b9fa6c 100644 --- a/uitest/src/com/vaadin/tests/tb3/TB3Runner.java +++ b/uitest/src/com/vaadin/tests/tb3/TB3Runner.java @@ -17,8 +17,10 @@ package com.vaadin.tests.tb3; import java.lang.annotation.Annotation; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedList; @@ -26,6 +28,8 @@ import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import org.apache.http.params.HttpConnectionParams; +import org.apache.http.params.HttpParams; import org.junit.Ignore; import org.junit.Test; import org.junit.runners.BlockJUnit4ClassRunner; @@ -34,6 +38,8 @@ import org.junit.runners.model.FrameworkMethod; import org.junit.runners.model.InitializationError; import org.junit.runners.model.Statement; import org.openqa.selenium.remote.DesiredCapabilities; +import org.openqa.selenium.remote.HttpCommandExecutor; +import org.openqa.selenium.remote.internal.HttpClientFactory; import com.vaadin.tests.annotations.TestCategory; import com.vaadin.tests.tb3.AbstractTB3Test.BrowserUtil; @@ -49,6 +55,13 @@ import com.vaadin.tests.tb3.MultiBrowserTest.Browser; */ public class TB3Runner extends BlockJUnit4ClassRunner { + /** + * Socket timeout for HTTP connections to the grid hub. The connection is + * closed after 15 minutes of inactivity to avoid builds hanging for up to + * three hours per connection if the test client crashes/hangs. + */ + private static final int SOCKET_TIMEOUT = 15 * 60 * 1000; + /** * This is the total limit of actual JUnit test instances run in parallel */ @@ -67,12 +80,34 @@ public class TB3Runner extends BlockJUnit4ClassRunner { MAX_CONCURRENT_TESTS = 50; } service = Executors.newFixedThreadPool(MAX_CONCURRENT_TESTS); + + // reduce socket timeout to avoid tests hanging for three hours + try { + Field field = HttpCommandExecutor.class + .getDeclaredField("httpClientFactory"); + assert (Modifier.isStatic(field.getModifiers())); + field.setAccessible(true); + field.set(null, new HttpClientFactory() { + @Override + public HttpParams getHttpParams() { + HttpParams params = super.getHttpParams(); + // fifteen minute timeout + HttpConnectionParams.setSoTimeout(params, SOCKET_TIMEOUT); + return params; + } + }); + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException( + "Changing socket timeout for TestBench failed", e); + } } protected static boolean localWebDriverIsUsed() { String useLocalWebDriver = System.getProperty("useLocalWebDriver"); - return useLocalWebDriver != null && useLocalWebDriver.toLowerCase().equals("true"); + return useLocalWebDriver != null + && useLocalWebDriver.toLowerCase().equals("true"); } public TB3Runner(Class klass) throws InitializationError { -- cgit v1.2.3 From e7632140cfe81062f1f81408c643e722661e60b5 Mon Sep 17 00:00:00 2001 From: Juuso Valli Date: Thu, 24 Jul 2014 10:54:42 +0300 Subject: Fix VWindow Vai-Aria roles for alertdialogs (#14289) Change-Id: Ie33ef684f2177fe1807f95bf234031cc3a44f317 --- client/src/com/vaadin/client/ui/VWindow.java | 8 +- .../tests/accessibility/WindowWaiAriaRoles.java | 107 +++++++++++++++++++++ .../accessibility/WindowWaiAriaRolesTest.java | 54 +++++++++++ 3 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRoles.java create mode 100644 uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRolesTest.java diff --git a/client/src/com/vaadin/client/ui/VWindow.java b/client/src/com/vaadin/client/ui/VWindow.java index 83a0001ad8..495e230156 100644 --- a/client/src/com/vaadin/client/ui/VWindow.java +++ b/client/src/com/vaadin/client/ui/VWindow.java @@ -233,7 +233,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /* * Stores the element that has focus in the application UI when the * window is opened, so it can be restored when the window closes. - * + * * This is currently implemented for the case when one non-modal window * can be open at the same time, and the focus is not changed while the * window is open. @@ -253,7 +253,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, /* * Restores the previously stored focused element. - * + * * When the focus was changed outside the window while the window was * open, the originally stored element is restored. */ @@ -640,7 +640,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * correctly if clicking on the "close" button in the window header but * closing the window from a button for example in the window will fail. * Symptom described in #10776 - * + * * The problematic part is that for the focus to be returned correctly * an input element needs to be focused in the root panel. Focusing some * other element apparently won't work. @@ -1411,7 +1411,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, * WAI-ARIA role to set for the window */ public void setWaiAriaRole(WindowRole role) { - if ("alertdialog".equals(role)) { + if (role == WindowRole.ALERTDIALOG) { Roles.getAlertdialogRole().set(getElement()); } else { Roles.getDialogRole().set(getElement()); diff --git a/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRoles.java b/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRoles.java new file mode 100644 index 0000000000..2ab6be25ac --- /dev/null +++ b/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRoles.java @@ -0,0 +1,107 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.accessibility; + +import java.util.Stack; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.window.WindowRole; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Window; + +/** + * UI to test if subwindows get the correct assistive roles. + * + * @author Vaadin Ltd + */ +public class WindowWaiAriaRoles extends AbstractTestUI { + Stack windows = new Stack(); + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server. + * VaadinRequest) + */ + @Override + protected void setup(VaadinRequest request) { + Button closeButton = new Button("Close windows"); + closeButton.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + while (!windows.isEmpty()) { + Window window = windows.pop(); + removeWindow(window); + } + } + + }); + + Button regularButton = new Button("Regular"); + regularButton.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + Window regularWindow = new Window("Regular window"); + openWindow(regularWindow); + } + }); + + Button alertButton = new Button("Alert"); + alertButton.addClickListener(new ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + Window alertWindow = new Window("Alert window"); + alertWindow.setAssistiveRole(WindowRole.ALERTDIALOG); + openWindow(alertWindow); + } + }); + addComponent(closeButton); + addComponent(regularButton); + addComponent(alertButton); + } + + void openWindow(Window window) { + windows.push(window); + window.center(); + addWindow(window); + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription() + */ + @Override + protected String getTestDescription() { + return "The alert window should have the role 'alertdialog' and the regular window should have the role 'dialog'"; + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber() + */ + @Override + protected Integer getTicketNumber() { + return 14289; + } + +} diff --git a/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRolesTest.java b/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRolesTest.java new file mode 100644 index 0000000000..e1d0452708 --- /dev/null +++ b/uitest/src/com/vaadin/tests/accessibility/WindowWaiAriaRolesTest.java @@ -0,0 +1,54 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.accessibility; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.WindowElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Test to see if regular and alert windows get the correct wai-aria roles + * + * @author Vaadin Ltd + */ +public class WindowWaiAriaRolesTest extends MultiBrowserTest { + + @Test + public void testRegularWindowRole() { + openTestURL(); + + $(ButtonElement.class).caption("Regular").first().click(); + String role = getWindowRole(); + Assert.assertTrue("Dialog has incorrect role '" + role + + "', expected 'dialog'", "dialog".equals(role)); + } + + @Test + public void testAlertWindowRole() { + openTestURL(); + $(ButtonElement.class).caption("Alert").first().click(); + String role = getWindowRole(); + Assert.assertTrue("Dialog has incorrect role '" + role + + "', expected 'alertdialog'", "alertdialog".equals(role)); + } + + public String getWindowRole() { + return $(WindowElement.class).first().getAttribute("role"); + } +} -- cgit v1.2.3 From 236293303bff740619a95131d5360bdbfe021c95 Mon Sep 17 00:00:00 2001 From: Bogdan Udrescu Date: Wed, 2 Jul 2014 18:29:56 +0300 Subject: TextArea size get reset when css resize is set (#14080) Listen to MouseUp event on the