From 89b6a437999773c2d7e283f2723c55ea9957710f Mon Sep 17 00:00:00 2001 From: Markus Koivisto Date: Thu, 31 Jul 2014 14:13:07 +0300 Subject: [PATCH] Revert "Keyboard shift-selection now works as expected in VScrollTable. (#14094)" This reverts commit 3390332b3e75e970cd3bf3b6bc5075bb867320b5. Conflicts: client/src/com/vaadin/client/ui/VScrollTable.java Change-Id: If33910705cdf9e4d7612557bfb55a01dde9e435d --- .../com/vaadin/client/ui/VScrollTable.java | 13 +++-- .../table/SelectAllConstantViewport.java | 6 +- .../table/SelectAllConstantViewportTest.java | 57 +------------------ .../vaadin/tests/tb3/MultiBrowserTest.java | 10 +--- 4 files changed, 14 insertions(+), 72 deletions(-) diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 6c807f0e02..2b8ffec2d7 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -1100,10 +1100,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets, * setRowFocus allows keyboard shift+downarrow * selection to work as expected. */ - if (isSingleSelectMode()) { - setRowFocus(row); - } + setRowFocus(row); } } if (selected != row.isSelected()) { @@ -7256,7 +7254,14 @@ public class VScrollTable extends FlowPanel implements HasWidgets, // Set new focused row focusedRow = row; - ensureRowIsVisible(row); + /* + * Don't scroll to the focused row when in multiselect mode. + * (#13341) + */ + + if (isSingleSelectMode()) { + 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 35ac63efe2..5a406eac48 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(178); + table.setCurrentPageFirstItemIndex(185); final CssLayout layout = new CssLayout(); layout.addComponent(selectAllCheckbox); @@ -82,9 +82,7 @@ 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. The scroll position should change if the user uses the keyboard to select " - + "multiple lines with shift+arrowkeys."; + return "The scroll position of a table with many items should remain constant if all items are selected."; } /* diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java index 257efef6a2..0e7a7c08a4 100644 --- a/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllConstantViewportTest.java @@ -15,20 +15,14 @@ */ 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; @@ -39,7 +33,7 @@ import com.vaadin.tests.tb3.MultiBrowserTest; public class SelectAllConstantViewportTest extends MultiBrowserTest { @Test - public void testViewportUnchangedwithMultiSel() throws IOException { + public void testViewportUnchanged() throws IOException { openTestURL(); CheckBoxElement checkbox = $(CheckBoxElement.class).first(); @@ -57,58 +51,11 @@ 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 ffa0b83dc2..ccbb6ca872 100644 --- a/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java +++ b/uitest/src/com/vaadin/tests/tb3/MultiBrowserTest.java @@ -41,8 +41,7 @@ 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()); @@ -51,13 +50,6 @@ 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 -- 2.39.5