Change focused Grid cell when scrolling with the keyboard (#18356).

- The focused cell is now updated when scrolling with pageup/down, home
or end key.
- The scroll amount is slightly reduced to ensure that no cells are
skipped over with pgup/down scroll.

Change-Id: Id1ddd4f0f3f3b1315df3f8d9813080fbd5009bcd

2 files changed, 64 insertions, 72 deletions

diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index f6772ba1f6..b3edca9140 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -2146,6 +2146,41 @@ public class Grid<T> extends ResizeComposite implements
                         return;
                     }
                     break;
+                case KeyCodes.KEY_HOME:
+                    if (newContainer.getRowCount() > 0) {
+                        newRow = 0;
+                    }
+                    break;
+                case KeyCodes.KEY_END:
+                    if (newContainer.getRowCount() > 0) {
+                        newRow = newContainer.getRowCount() - 1;
+                    }
+                    break;
+                case KeyCodes.KEY_PAGEDOWN:
+                case KeyCodes.KEY_PAGEUP:
+                    if (newContainer.getRowCount() > 0) {
+                        boolean down = event.getKeyCode() == KeyCodes.KEY_PAGEDOWN;
+                        // If there is a visible focused cell, scroll by one
+                        // page from its position. Otherwise, use the first or
+                        // the last visible row as the scroll start position.
+                        // This avoids jumping when using both keyboard and the
+                        // scroll bar for scrolling.
+                        int firstVisible = getFirstVisibleRowIndex();
+                        int lastVisible = getLastVisibleRowIndex();
+                        if (newRow < firstVisible || newRow > lastVisible) {
+                            newRow = down ? lastVisible : firstVisible;
+                        }
+                        // Scroll by a little less than the visible area to
+                        // account for the possibility that the top and the
+                        // bottom row are only partially visible.
+                        int moveFocusBy = Math.max(1, lastVisible
+                                - firstVisible - 1);
+                        moveFocusBy *= down ? 1 : -1;
+                        newRow += moveFocusBy;
+                        newRow = Math.max(0, Math.min(
+                                newContainer.getRowCount() - 1, newRow));
+                    }
+                    break;
                 default:
                     return;
                 }
@@ -6376,10 +6411,6 @@ public class Grid<T> extends ResizeComposite implements
                 return;
             }
 
-            if (handleNavigationEvent(event, container)) {
-                return;
-            }
-
             if (handleCellFocusEvent(event, container)) {
                 return;
             }
@@ -6518,56 +6549,6 @@ public class Grid<T> extends ResizeComposite implements
         return false;
     }
 
-    private boolean handleNavigationEvent(Event event, RowContainer unused) {
-        if (!event.getType().equals(BrowserEvents.KEYDOWN)) {
-            // Only handle key downs
-            return false;
-        }
-
-        int newRow = -1;
-        RowContainer container = escalator.getBody();
-        switch (event.getKeyCode()) {
-        case KeyCodes.KEY_HOME:
-            if (container.getRowCount() > 0) {
-                newRow = 0;
-            }
-            break;
-        case KeyCodes.KEY_END:
-            if (container.getRowCount() > 0) {
-                newRow = container.getRowCount() - 1;
-            }
-            break;
-        case KeyCodes.KEY_PAGEUP: {
-            Range range = escalator.getVisibleRowRange();
-            if (!range.isEmpty()) {
-                int firstIndex = getFirstVisibleRowIndex();
-                newRow = firstIndex - range.length();
-                if (newRow < 0) {
-                    newRow = 0;
-                }
-            }
-            break;
-        }
-        case KeyCodes.KEY_PAGEDOWN: {
-            Range range = escalator.getVisibleRowRange();
-            if (!range.isEmpty()) {
-                int lastIndex = getLastVisibleRowIndex();
-                newRow = lastIndex + range.length();
-                if (newRow >= container.getRowCount()) {
-                    newRow = container.getRowCount() - 1;
-                }
-            }
-            break;
-        }
-        default:
-            return false;
-        }
-
-        scrollToRow(newRow);
-
-        return true;
-    }
-
     private boolean handleHeaderCellDragStartEvent(Event event,
             RowContainer container) {
         if (!isColumnReorderingAllowed()) {
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridKeyboardNavigationTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridKeyboardNavigationTest.java
index 3f2e82793b..6724a7bb49 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridKeyboardNavigationTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridKeyboardNavigationTest.java
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 
 import com.vaadin.testbench.elements.GridElement;
@@ -192,30 +193,40 @@ public class GridKeyboardNavigationTest extends GridBasicFeaturesTest {
 
         selectMenuPath("Component", "Size", "HeightMode Row");
 
-        getGridElement().getCell(5, 2).click();
-
+        getGridElement().getCell(9, 2).click();
         new Actions(getDriver()).sendKeys(Keys.PAGE_DOWN).perform();
-        assertTrue("Row 20 did not become visible",
-                isElementPresent(By.xpath("//td[text() = '(20, 2)']")));
+        assertTrue("Row 17 did not become visible",
+                isElementPresent(By.xpath("//td[text() = '(17, 2)']")));
 
         new Actions(getDriver()).sendKeys(Keys.PAGE_DOWN).perform();
-        assertTrue("Row 30 did not become visible",
-                isElementPresent(By.xpath("//td[text() = '(30, 2)']")));
-
-        assertTrue("Originally focused cell is no longer focused",
-                getGridElement().getCell(5, 2).isFocused());
-
-        getGridElement().getCell(50, 2).click();
+        assertTrue("Row 25 did not become visible",
+                isElementPresent(By.xpath("//td[text() = '(25, 2)']")));
+        checkFocusedCell(29, 2, 4);
 
+        getGridElement().getCell(41, 2).click();
         new Actions(getDriver()).sendKeys(Keys.PAGE_UP).perform();
-        assertTrue("Row 31 did not become visible",
-                isElementPresent(By.xpath("//td[text() = '(31, 2)']")));
+        assertTrue("Row 33 did not become visible",
+                isElementPresent(By.xpath("//td[text() = '(33, 2)']")));
 
         new Actions(getDriver()).sendKeys(Keys.PAGE_UP).perform();
-        assertTrue("Row 21 did not become visible",
-                isElementPresent(By.xpath("//td[text() = '(21, 2)']")));
+        assertTrue("Row 25 did not become visible",
+                isElementPresent(By.xpath("//td[text() = '(25, 2)']")));
+        checkFocusedCell(21, 2, 4);
+    }
 
-        assertTrue("Originally focused cell is no longer focused",
-                getGridElement().getCell(50, 2).isFocused());
+    private void checkFocusedCell(int row, int column, int rowTolerance) {
+        WebElement focusedCell = getGridElement().findElement(
+                By.className("v-grid-cell-focused"));
+        String cellContents = focusedCell.getText();
+        String[] rowAndCol = cellContents.replaceAll("[()\\s]", "").split(",");
+        int focusedRow = Integer.parseInt(rowAndCol[0].trim());
+        int focusedColumn = Integer.parseInt(rowAndCol[1].trim());
+        // rowTolerance is the maximal allowed difference from the expected
+        // focused row. It is required because scrolling using page up/down
+        // may not move the position by exactly the visible height of the grid.
+        assertTrue("The wrong cell is focused. Expected (" + row + "," + column
+                + "), was " + cellContents,
+                column == focusedColumn
+                        && Math.abs(row - focusedRow) <= rowTolerance);
     }
-}
+}
\ No newline at end of file