Fix scrolling bug when moving Grid editor upsnapshot/7.6

Change-Id: I005e17ec8ce7927e5fc36975b1e784444d5904b7

3 files changed, 56 insertions, 33 deletions

diff --git a/client/src/com/vaadin/client/widget/escalator/RowVisibilityChangeEvent.java b/client/src/com/vaadin/client/widget/escalator/RowVisibilityChangeEvent.java
index 968013b401..056efd9915 100644
--- a/client/src/com/vaadin/client/widget/escalator/RowVisibilityChangeEvent.java
+++ b/client/src/com/vaadin/client/widget/escalator/RowVisibilityChangeEvent.java
@@ -17,6 +17,7 @@
 package com.vaadin.client.widget.escalator;
 
 import com.google.gwt.event.shared.GwtEvent;
+import com.vaadin.shared.ui.grid.Range;
 
 /**
  * Event fired when the range of visible rows changes e.g. because of scrolling.
@@ -31,8 +32,7 @@ public class RowVisibilityChangeEvent extends
      */
     public static final Type<RowVisibilityChangeHandler> TYPE = new Type<RowVisibilityChangeHandler>();
 
-    private final int firstVisibleRow;
-    private final int visibleRowCount;
+    private final Range visibleRows;
 
     /**
      * Creates a new row visibility change event
@@ -43,8 +43,7 @@ public class RowVisibilityChangeEvent extends
      *            the number of visible rows
      */
     public RowVisibilityChangeEvent(int firstVisibleRow, int visibleRowCount) {
-        this.firstVisibleRow = firstVisibleRow;
-        this.visibleRowCount = visibleRowCount;
+        visibleRows = Range.withLength(firstVisibleRow, visibleRowCount);
     }
 
     /**
@@ -53,7 +52,7 @@ public class RowVisibilityChangeEvent extends
      * @return the index of the first visible row
      */
     public int getFirstVisibleRow() {
-        return firstVisibleRow;
+        return visibleRows.getStart();
     }
 
     /**
@@ -62,7 +61,17 @@ public class RowVisibilityChangeEvent extends
      * @return the number of visible rows
      */
     public int getVisibleRowCount() {
-        return visibleRowCount;
+        return visibleRows.length();
+    }
+
+    /**
+     * Gets the range of visible rows.
+     * 
+     * @since
+     * @return the visible rows
+     */
+    public Range getVisibleRowRange() {
+        return visibleRows;
     }
 
     /*
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 449cb60cc7..e9ee7d71b2 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -1294,16 +1294,8 @@ public class Grid<T> extends ResizeComposite implements
         private int focusedColumnIndex = -1;
         private String styleName = null;
 
-        /*
-         * Used to track Grid horizontal scrolling
-         */
-        private HandlerRegistration scrollHandler;
-
-        /*
-         * Used to open editor once Grid has vertically scrolled to the proper
-         * position and data is available
-         */
-        private HandlerRegistration dataAvailableHandler;
+        private HandlerRegistration hScrollHandler;
+        private HandlerRegistration vScrollHandler;
 
         private final Button saveButton;
         private final Button cancelButton;
@@ -1506,21 +1498,22 @@ public class Grid<T> extends ResizeComposite implements
             this.focusedColumnIndex = columnIndex;
             state = State.ACTIVATING;
 
-            if (grid.getEscalator().getVisibleRowRange().contains(rowIndex)) {
+            final Escalator escalator = grid.getEscalator();
+            if (escalator.getVisibleRowRange().contains(rowIndex)) {
                 show(rowIndex);
             } else {
-                hideOverlay();
-                dataAvailableHandler = grid
-                        .addDataAvailableHandler(new DataAvailableHandler() {
-                            @Override
-                            public void onDataAvailable(DataAvailableEvent event) {
-                                if (event.getAvailableRows().contains(rowIndex)) {
-                                    show(rowIndex);
-                                    dataAvailableHandler.removeHandler();
-                                }
-                            }
-                        });
-                grid.scrollToRow(rowIndex, ScrollDestination.MIDDLE);
+                vScrollHandler = grid.addScrollHandler(new ScrollHandler() {
+                    @Override
+                    public void onScroll(ScrollEvent event) {
+                        if (escalator.getVisibleRowRange().contains(rowIndex)) {
+                            show(rowIndex);
+                            vScrollHandler.removeHandler();
+                        }
+                    }
+                });
+                grid.scrollToRow(rowIndex,
+                        isBuffered() ? ScrollDestination.MIDDLE
+                                : ScrollDestination.ANY);
             }
         }
 
@@ -1718,7 +1711,7 @@ public class Grid<T> extends ResizeComposite implements
             TableRowElement tr = grid.getEscalator().getBody()
                     .getRowElement(rowIndex);
 
-            scrollHandler = grid.addScrollHandler(new ScrollHandler() {
+            hScrollHandler = grid.addScrollHandler(new ScrollHandler() {
                 @Override
                 public void onScroll(ScrollEvent event) {
                     updateHorizontalScrollPosition();
@@ -1932,7 +1925,7 @@ public class Grid<T> extends ResizeComposite implements
             frozenCellWrapper.removeAllChildren();
             editorOverlay.removeFromParent();
 
-            scrollHandler.removeHandler();
+            hScrollHandler.removeHandler();
 
             clearEditorColumnErrors();
 
@@ -3730,7 +3723,7 @@ public class Grid<T> extends ResizeComposite implements
 
         @Override
         public void setEnabled(boolean enabled) {
-            if(!enabled && isOpen()) {
+            if (!enabled && isOpen()) {
                 close();
             }
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java
index 0b84d3470f..4fe88c6eac 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java
@@ -66,7 +66,7 @@ public class GridEditorUnbufferedTest extends GridEditorTest {
     }
 
     @Test
-    public void testEditorMove() {
+    public void testEditorMoveWithMouse() {
         selectMenuPath(EDIT_ITEM_5);
 
         assertEditorOpen();
@@ -82,6 +82,27 @@ public class GridEditorUnbufferedTest extends GridEditorTest {
     }
 
     @Test
+    public void testEditorMoveWithKeyboard() {
+        selectMenuPath(EDIT_ITEM_100);
+
+        assertEditorOpen();
+
+        getGridElement().sendKeys(Keys.ENTER);
+
+        String firstFieldValue = getEditorWidgets().get(0)
+                .getAttribute("value");
+        assertEquals("Editor should move to row 101", "(101, 0)",
+                firstFieldValue);
+
+        for (int i = 0; i < 10; i++) {
+            getGridElement().sendKeys(Keys.SHIFT, Keys.ENTER);
+        }
+
+        firstFieldValue = getEditorWidgets().get(0).getAttribute("value");
+        assertEquals("Editor should move to row 91", "(91, 0)", firstFieldValue);
+    }
+
+    @Test
     public void testValidationErrorPreventsMove() {
         // Because of "out of view" issues, we need to move this for easy access
         selectMenuPath("Component", "Columns", "Column 7", "Column 7 Width",