Fix Table pageup/pagedown navigation (#15332)

After this patch navigation with PageDown, PageUp, Home, End works
properly. What was
changed in this patch (new state): the first visible row number is
updated before
sending selected rows to the server while handling these keys.

Change-Id: I3bdebc434f886ef55f90f3fed5fd607d5f65f87f

client/src/com/vaadin/client/ui/VScrollTable.java

@@ -1055,6 +1055,11 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         sendSelectedRows(immediate);
     }
 
+    private void updateFirstVisibleAndSendSelectedRows() {
+        updateFirstVisibleRow();
+        sendSelectedRows(immediate);
+    }
+
     /**
      * Sends the selection to the server if it has been changed since the last
      * update/visit.
@@ -7232,6 +7237,20 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         return s;
     }
 
+    // Updates first visible row for the case we cannot wait
+    // for onScroll
+    private void updateFirstVisibleRow() {
+        scrollTop = scrollBodyPanel.getScrollPosition();
+        firstRowInViewPort = calcFirstRowInViewPort();
+        int maxFirstRow = totalRows - pageLength;
+        if (firstRowInViewPort > maxFirstRow && maxFirstRow >= 0) {
+            firstRowInViewPort = maxFirstRow;
+        }
+        lastRequestedFirstvisible = firstRowInViewPort;
+        client.updateVariable(paintableId, "firstvisible", firstRowInViewPort,
+                false);
+    }
+
     /**
      * This method has logic which rows needs to be requested from server when
      * user scrolls
@@ -7700,7 +7719,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         // focus and select the last visible row
                         setRowFocus(lastVisibleRowInViewPort);
                         selectFocusedRow(ctrl, shift);
-                        sendSelectedRows();
+                        updateFirstVisibleAndSendSelectedRows();
                     } else {
                         int indexOfToBeFocused = focusedRow.getIndex()
                                 + getFullyVisibleRowCount();
@@ -7717,7 +7736,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                             setRowFocus(toBeFocusedRow);
                             selectFocusedRow(ctrl, shift);
                             // TODO needs scrollintoview ?
-                            sendSelectedRows();
+                            updateFirstVisibleAndSendSelectedRows();
                         } else {
                             // scroll down by pixels and return, to wait for
                             // new rows, then select the last item in the
@@ -7753,7 +7772,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         // focus and select the first visible row
                         setRowFocus(firstVisibleRowInViewPort);
                         selectFocusedRow(ctrl, shift);
-                        sendSelectedRows();
+                        updateFirstVisibleAndSendSelectedRows();
                     } else {
                         int indexOfToBeFocused = focusedRow.getIndex()
                                 - getFullyVisibleRowCount();
@@ -7768,7 +7787,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                             setRowFocus(toBeFocusedRow);
                             selectFocusedRow(ctrl, shift);
                             // TODO needs scrollintoview ?
-                            sendSelectedRows();
+                            updateFirstVisibleAndSendSelectedRows();
                         } else {
                             // unless waiting for the next rowset already
                             // scroll down by pixels and return, to wait for
@@ -7800,7 +7819,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     if (rowByRowIndex.getIndex() == 0) {
                         setRowFocus(rowByRowIndex);
                         selectFocusedRow(ctrl, shift);
-                        sendSelectedRows();
+                        updateFirstVisibleAndSendSelectedRows();
                     } else {
                         // first row of table will come in next row fetch
                         if (ctrl) {
@@ -7826,7 +7845,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     if (focusedRow != rowByRowIndex) {
                         setRowFocus(rowByRowIndex);
                         selectFocusedRow(ctrl, shift);
-                        sendSelectedRows();
+                        updateFirstVisibleAndSendSelectedRows();
                     }
                 } else {
                     if (ctrl) {

uitest/src/com/vaadin/tests/components/table/TableNavigationPageDown.java

@@ -0,0 +1,56 @@
+/*
+ * 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 com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Table;
+
+public class TableNavigationPageDown extends AbstractTestUI {
+
+    private final static int ROW_NUMBER = 50;
+
+    @Override
+    protected void setup(VaadinRequest req) {
+        Table table = new Table();
+        table.setSelectable(true);
+        table.setImmediate(true);
+        table.setHeight("150px");
+
+        table.addContainerProperty("num", Integer.class, "num");
+        table.addContainerProperty("Foo", String.class, "Foov");
+        table.addContainerProperty("Bar", String.class, "Barv");
+
+        for (int i = 0; i < ROW_NUMBER; i++) {
+            Object key = table.addItem();
+            table.getItem(key).getItemProperty("num").setValue(i);
+        }
+
+        addComponent(table);
+
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Navigation in Table with PageDown/PageUp/Home/End keys should work";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 15332;
+    }
+
+}

uitest/src/com/vaadin/tests/components/table/TableNavigationPageDownTest.java

@@ -0,0 +1,223 @@
+/*
+ * 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 static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+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.elements.TableElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Tests that navigation with PageDown/PageUp/Home/End in Table works
+ * 
+ * @author Vaadin Ltd
+ */
+public class TableNavigationPageDownTest extends MultiBrowserTest {
+
+    private static final int ROW_NUMBER = 50;
+    private int lowerWrapperY = -1;
+    private int pageHeight = -1;
+    private int rowHeight = -1;
+    private int lastRowNumber = -1;
+
+    private WebElement lastRow;
+    private WebElement wrapper;
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+
+        TableElement table = $(TableElement.class).first();
+        rowHeight = table.getCell(1, 0).getLocation().getY()
+                - table.getCell(0, 0).getLocation().getY();
+
+        wrapper = findElement(By.className("v-table-body-wrapper"));
+        pageHeight = wrapper.getSize().getHeight();
+        lowerWrapperY = wrapper.getLocation().getY() + pageHeight;
+    }
+
+    @Test
+    public void navigatePageDown() {
+        // Scroll to a point where you can reach the bottom with 3 PageDowns.
+        // Can't use v-table-body height because lower rows haven't been
+        // fetched yet.
+        testBenchElement(wrapper).scroll(
+                ROW_NUMBER * rowHeight - (int) (2.8 * pageHeight));
+        waitForScrollToFinish();
+
+        lastRow = getLastVisibleRow();
+        lastRow.click();
+
+        // Press PageDown 3 times, we should reach the last row
+        for (int j = 0; j < 3; j++) {
+            lastRowNumber = getRowNumber(lastRow);
+            new Actions(driver).sendKeys(Keys.PAGE_DOWN).build().perform();
+            waitForRowsToUpdate();
+        }
+        assertEquals("Last table row should be visible", ROW_NUMBER - 1,
+                lastRowNumber);
+    }
+
+    @Test
+    public void navigatePageUp() {
+        // Scroll to a point where you can reach the top with 3 PageUps.
+        testBenchElement(wrapper).scroll((int) (2.8 * pageHeight));
+        waitForScrollToFinish();
+
+        lastRow = getLastVisibleRow();
+        getFirstVisibleRow().click();
+
+        // Press PageUp 3 times, we should reach the first row
+        for (int j = 0; j < 3; j++) {
+            lastRowNumber = getRowNumber(lastRow);
+            new Actions(driver).sendKeys(Keys.PAGE_UP).build().perform();
+            waitForRowsToUpdate();
+        }
+        assertEquals("First table row should be visible", 0,
+                getRowNumber(getFirstVisibleRow()));
+    }
+
+    @Test
+    public void navigateEndAndHome() {
+        lastRow = getLastVisibleRow();
+        lastRow.click();
+
+        new Actions(driver).sendKeys(Keys.END).build().perform();
+        waitForScrollToFinish();
+
+        assertEquals("Last table row should be visible", ROW_NUMBER - 1,
+                getRowNumber(getLastVisibleRow()));
+
+        new Actions(driver).sendKeys(Keys.HOME).build().perform();
+        waitForScrollToFinish();
+
+        assertEquals("First table row should be visible", 0,
+                getRowNumber(getFirstVisibleRow()));
+    }
+
+    /**
+     * Waits until the scroll position indicator goes away, signifying that all
+     * the required rows have been fetched.
+     */
+    private void waitForScrollToFinish() {
+        waitUntil(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                List<WebElement> elements = findElements(By
+                        .className("v-table-scrollposition"));
+                return elements.isEmpty() || !elements.get(0).isDisplayed();
+            }
+
+            @Override
+            public String toString() {
+                // Timed out after 10 seconds waiting for ...
+                return "scroll position indicator to vanish";
+            }
+        });
+    }
+
+    /**
+     * Waits until the visible rows have been updated to some other set of rows.
+     * Fails if there is no change.
+     */
+    private void waitForRowsToUpdate() {
+        waitUntil(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                lastRow = getLastVisibleRow();
+                int rowNumber = getRowNumber(lastRow);
+                if (lastRowNumber != rowNumber) {
+                    lastRowNumber = rowNumber;
+                    return true;
+                }
+                return false;
+            }
+
+            @Override
+            public String toString() {
+                // Timed out after 10 seconds waiting for ...
+                return "visible rows to be updated";
+            }
+        });
+    }
+
+    /**
+     * Returns row number from its first cell
+     */
+    private int getRowNumber(WebElement row) {
+        return Integer.valueOf(row.findElement(
+                By.className("v-table-cell-wrapper")).getText());
+    }
+
+    /**
+     * Returns the first fully visible row
+     */
+    private WebElement getFirstVisibleRow() {
+        List<WebElement> allFetchedRows = wrapper
+                .findElements(By.tagName("tr"));
+        int wrapperY = wrapper.getLocation().getY();
+        for (WebElement row : allFetchedRows) {
+            int rowY = row.getLocation().getY();
+            if ((rowY >= wrapperY) && (rowY - rowHeight <= wrapperY)) {
+                return row;
+            }
+        }
+        fail("Could not find first visible row");
+        return null;
+    }
+
+    /**
+     * Returns the last fully visible row
+     */
+    private WebElement getLastVisibleRow() {
+        List<WebElement> allFetchedRows = wrapper
+                .findElements(By.tagName("tr"));
+        for (WebElement row : allFetchedRows) {
+            int lowerY = row.getLocation().getY() + rowHeight;
+            if ((lowerY <= lowerWrapperY)
+                    && (lowerY + rowHeight >= lowerWrapperY)) {
+                return row;
+            }
+        }
+        fail("Could not find last visible row");
+        return null;
+    }
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        // Sending PageDown has no effect on PhantomJS. On IE focus
+        // in Table is often lost, so default scrolling happens on PageDown.
+        List<DesiredCapabilities> browsers = new ArrayList<DesiredCapabilities>(
+                getBrowsersExcludingIE());
+        browsers.remove(Browser.PHANTOMJS.getDesiredCapabilities());
+        return browsers;
+    }
+
+}