From 04d793d80505f8c8274800e2620a2186ae32409d Mon Sep 17 00:00:00 2001 From: Guillermo Alvarez Date: Fri, 12 Sep 2014 15:49:53 +0300 Subject: [PATCH] Fix multiselection pressing shift before starting (#13483) selectionRangeStart wasn't set when starting a selection pressing shift. SelectAllRowsTest was rewritten to enable extension to test this issue. Change-Id: I1b578b28ba89fc8215ec853d92de09f44c2d58e6 --- .../com/vaadin/client/ui/VScrollTable.java | 1 + .../tests/components/table/SelectAllRows.java | 11 +-- .../table/SelectAllRowsShiftFirst.java | 30 +++++++ .../table/SelectAllRowsShiftFirstTest.java | 36 ++++++++ .../components/table/SelectAllRowsTest.java | 85 +++++++++++-------- 5 files changed, 118 insertions(+), 45 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java create mode 100644 uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 2e3c110d43..859d600daf 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -6439,6 +6439,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, VScrollTableRow startRow = selectionRangeStart; if (startRow == null) { startRow = focusedRow; + selectionRangeStart = focusedRow; // If start row is null then we have a multipage selection // from // above diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java index 6cc6a68a7d..4778bca3b5 100644 --- a/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java @@ -22,7 +22,6 @@ import com.vaadin.tests.components.AbstractTestUI; import com.vaadin.ui.Button; import com.vaadin.ui.Label; import com.vaadin.ui.Table; -import com.vaadin.ui.VerticalLayout; public class SelectAllRows extends AbstractTestUI { @@ -33,10 +32,6 @@ public class SelectAllRows extends AbstractTestUI { @Override protected void setup(VaadinRequest request) { - VerticalLayout layout = new VerticalLayout(); - layout.setMargin(true); - layout.setSpacing(true); - setContent(layout); final Table table = new Table(); table.setId(TABLE); @@ -44,16 +39,16 @@ public class SelectAllRows extends AbstractTestUI { table.setMultiSelect(true); table.setSelectable(true); table.addContainerProperty("row", String.class, null); - layout.addComponent(table); + addComponent(table); Button button = new Button("Count"); button.setId(COUNT_SELECTED_BUTTON); - layout.addComponent(button); + addComponent(button); final Label label = new Label(); label.setId(COUNT_OF_SELECTED_ROWS_LABEL); label.setCaption("Selected count:"); - layout.addComponent(label); + addComponent(label); button.addClickListener(new Button.ClickListener() { diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java new file mode 100644 index 0000000000..8d3e922fa9 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java @@ -0,0 +1,30 @@ +/* + * 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.table; + +public class SelectAllRowsShiftFirst extends SelectAllRows { + + @Override + protected String getTestDescription() { + return "Selecting all rows does not work by pressing shift and selecting the first row, and then press shift then select last row"; + } + + @Override + protected Integer getTicketNumber() { + return 13483; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java new file mode 100644 index 0000000000..28cf9b0718 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java @@ -0,0 +1,36 @@ +/* + * 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.table; + +import java.util.List; + +import org.openqa.selenium.WebElement; + +/** + * Test to see if all items of the table can be selected by pressing shift and + * selecting the first row, and then press shift then select last row (#13483) + * + * @author Vaadin Ltd + */ +public class SelectAllRowsShiftFirstTest extends SelectAllRowsTest { + + @Override + protected void clickFirstRow() { + List rows = getVisibleTableRows(); + shiftClickElement(rows.get(0)); + } + +} diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java index d4e8441757..f6c6ca3ddc 100644 --- a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java +++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java @@ -15,9 +15,6 @@ */ package com.vaadin.tests.components.table; -import static com.vaadin.tests.components.table.SelectAllRows.COUNT_OF_SELECTED_ROWS_LABEL; -import static com.vaadin.tests.components.table.SelectAllRows.COUNT_SELECTED_BUTTON; -import static com.vaadin.tests.components.table.SelectAllRows.TABLE; import static com.vaadin.tests.components.table.SelectAllRows.TOTAL_NUMBER_OF_ROWS; import static org.junit.Assert.assertEquals; @@ -27,12 +24,24 @@ import java.util.List; import org.junit.Test; import org.openqa.selenium.By; import org.openqa.selenium.Keys; +import org.openqa.selenium.NoSuchElementException; +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.elements.ButtonElement; +import com.vaadin.testbench.elements.LabelElement; +import com.vaadin.testbench.elements.TableElement; import com.vaadin.tests.tb3.MultiBrowserTest; +/** + * Test to see if all items of the table can be selected by selecting first row, + * press shift then select last (#13008) + * + * @author Vaadin Ltd + */ public class SelectAllRowsTest extends MultiBrowserTest { @Override @@ -54,56 +63,58 @@ public class SelectAllRowsTest extends MultiBrowserTest { public void testAllRowsAreSelected() { openTestURL(); - selectAllRowsInTable(); - int selectedRows = countSelectedItems(); + clickFirstRow(); + scrollTableToBottom(); + clickLastRow(); - assertEquals(TOTAL_NUMBER_OF_ROWS, selectedRows); + assertEquals(TOTAL_NUMBER_OF_ROWS, countSelectedItems()); } - private int countSelectedItems() { - WebElement countButton = vaadinElementById(COUNT_SELECTED_BUTTON); - countButton.click(); - WebElement countOfSelectedRows = vaadinElementById(COUNT_OF_SELECTED_ROWS_LABEL); - String count = countOfSelectedRows.getText(); - return Integer.parseInt(count); + protected void clickFirstRow() { + getVisibleTableRows().get(0).click(); } - private void selectAllRowsInTable() { - clickFirstRow(); - scrollTableToBottom(); - new Actions(getDriver()).keyDown(Keys.SHIFT).click(getLastRow()) + private void clickLastRow() { + List rows = getVisibleTableRows(); + shiftClickElement(rows.get(rows.size() - 1)); + } + + protected void shiftClickElement(WebElement element) { + new Actions(getDriver()).keyDown(Keys.SHIFT).click(element) .keyUp(Keys.SHIFT).perform(); } - private WebElement getLastRow() { - List rows = allVisibleTableRows(); - WebElement lastRow = rows.get(rows.size() - 1); - return lastRow; + private int countSelectedItems() { + $(ButtonElement.class).first().click(); + String count = $(LabelElement.class).get(1).getText(); + return Integer.parseInt(count); } - private void clickFirstRow() { - WebElement firstRow = allVisibleTableRows().get(0); - firstRow.click(); + private TableElement getTable() { + return $(TableElement.class).first(); } private void scrollTableToBottom() { - WebElement table = vaadinElementById(TABLE); - testBenchElement(table.findElement(By.className("v-scrollable"))) + testBenchElement(getTable().findElement(By.className("v-scrollable"))) .scroll(TOTAL_NUMBER_OF_ROWS * 30); + waitUntilRowIsVisible(TOTAL_NUMBER_OF_ROWS - 1); + } - // Wait for scrolling to complete. Otherwise, clicking last row will - // fail with Chrome - try { - Thread.sleep(200); - } catch (InterruptedException e) { - e.printStackTrace(); - } + private void waitUntilRowIsVisible(final int row) { + waitUntil(new ExpectedCondition() { + @Override + public Object apply(WebDriver input) { + try { + return getTable().getCell(row, 0) != null; + } catch (NoSuchElementException e) { + return false; + } + } + }); } - private List allVisibleTableRows() { - WebElement table = vaadinElementById(TABLE); - List rows = table.findElements(By - .cssSelector(".v-table-table tr")); - return rows; + protected List getVisibleTableRows() { + return getTable().findElements(By.cssSelector(".v-table-table tr")); } + } -- 2.39.5