Change-Id: I0dcd9f75cd30fe91c17ca0755241e73a37da79ectags/7.3.0.rc1
@@ -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; | |||
} |
@@ -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."; | |||
} | |||
/* |
@@ -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<Boolean>() { | |||
@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<DesiredCapabilities> getBrowsersToTest() { | |||
// phantomJS does not support keyboard actions | |||
return getBrowsersExcludingPhantomJS(); | |||
} | |||
} |
@@ -41,7 +41,8 @@ import org.openqa.selenium.remote.DesiredCapabilities; | |||
public abstract class MultiBrowserTest extends PrivateTB3Configuration { | |||
protected List<DesiredCapabilities> getBrowsersExcludingIE() { | |||
List<DesiredCapabilities> browsers = new ArrayList<DesiredCapabilities>(getAllBrowsers()); | |||
List<DesiredCapabilities> browsers = new ArrayList<DesiredCapabilities>( | |||
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<DesiredCapabilities> getBrowsersExcludingPhantomJS() { | |||
List<DesiredCapabilities> browsers = new ArrayList<DesiredCapabilities>( | |||
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 |