Simplify Grid scroll handling. (#11835)

If first attempt at scrolling doesn't succeed it's unlikely that
continuing to wait is going to make any difference. Cache should be
populated before triggering any actions that depend on the row being
visible, otherwise it should be enough to trust that scrollToRow
actually scrolls to row and once scrolling is done the row is as much in
view as it's going to get. This way we don't get into a situation where
Editor never opens because it's still waiting for that one last pixel
that can't be achieved thanks to browser zoom causing rounding errors.

Continues on #11672

3 files changed, 65 insertions, 46 deletions

diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java
index 74a584aa5b..93d2c9189a 100644
--- a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java
@@ -93,9 +93,9 @@ public class GridConnector extends AbstractListingConnector
      * The scrolling methods must trigger the scrolling only after any potential
      * resizing or other similar action triggered from the server side within
      * the same round trip has had a chance to happen, so there needs to be a
-     * delay. The delay is done with <code>scheduleFinally</code> rather than
-     * <code>scheduleDeferred</code> because the latter has been known to cause
-     * flickering in Grid.
+     * delay. The delay is done with <code>scheduleDeferred</code> rather than
+     * <code>scheduleFinally</code> because otherwise the order of the
+     * operations isn't guaranteed.
      *
      */
     private class GridConnectorClientRpc implements GridClientRpc {
@@ -107,7 +107,7 @@ public class GridConnector extends AbstractListingConnector
 
         @Override
         public void scrollToRow(int row, ScrollDestination destination) {
-            Scheduler.get().scheduleFinally(() -> {
+            Scheduler.get().scheduleDeferred(() -> {
                 grid.scrollToRow(row, destination);
                 // Add details refresh listener and handle possible detail
                 // for scrolled row.
@@ -121,12 +121,12 @@ public class GridConnector extends AbstractListingConnector
 
         @Override
         public void scrollToStart() {
-            Scheduler.get().scheduleFinally(() -> grid.scrollToStart());
+            Scheduler.get().scheduleDeferred(() -> grid.scrollToStart());
         }
 
         @Override
         public void scrollToEnd() {
-            Scheduler.get().scheduleFinally(() -> {
+            Scheduler.get().scheduleDeferred(() -> {
                 grid.scrollToEnd();
                 addDetailsRefreshCallback(() -> {
                     if (rowHasDetails(grid.getDataSource().size() - 1)) {
diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
index e98ae2c578..7ad1cabf00 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -7713,14 +7713,11 @@ public class Escalator extends Widget
     public void scrollToRowAndSpacer(final int rowIndex,
             final ScrollDestination destination, final int padding)
             throws IllegalArgumentException {
-        // wait for the layout phase to finish
-        Scheduler.get().scheduleFinally(() -> {
-            if (rowIndex != -1) {
-                verifyValidRowIndex(rowIndex);
-            }
-            body.scrollToRowSpacerOrBoth(rowIndex, destination, padding,
-                    ScrollType.ROW_AND_SPACER);
-        });
+        if (rowIndex != -1) {
+            verifyValidRowIndex(rowIndex);
+        }
+        body.scrollToRowSpacerOrBoth(rowIndex, destination, padding,
+                ScrollType.ROW_AND_SPACER);
     }
 
     private static void validateScrollDestination(
diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index 977935c9bf..2d1cc08902 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -1694,9 +1694,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             }
             state = State.ACTIVATING;
 
-            grid.scrollToRow(rowIndex,
-                    isBuffered() ? ScrollDestination.MIDDLE
-                            : ScrollDestination.ANY,
+            grid.scrollToRow(rowIndex, ScrollDestination.ANY,
                     () -> show(rowIndex, columnIndexDOM));
         }
 
@@ -7459,15 +7457,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      */
     public void scrollToRow(int rowIndex, ScrollDestination destination,
             Runnable callback) {
-        waitUntilVisible(rowIndex, destination, () -> {
-            Reference<HandlerRegistration> registration = new Reference<>();
-            registration.set(addDataAvailableHandler(event -> {
-                if (event.getAvailableRows().contains(rowIndex)) {
-                    registration.get().removeHandler();
-                    callback.run();
-                }
-            }));
-        });
+        waitUntilVisible(rowIndex, destination, callback);
     }
 
     /**
@@ -7533,29 +7523,61 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      */
     private void waitUntilVisible(int rowIndex, ScrollDestination destination,
             Runnable whenVisible) {
-        final Escalator escalator = getEscalator();
-        if (escalator.getVisibleRowRange().contains(rowIndex)) {
-            TableRowElement rowElement = escalator.getBody()
-                    .getRowElement(rowIndex);
-            long bottomBorder = Math.round(WidgetUtil.getBorderBottomThickness(
-                    rowElement.getFirstChildElement()) + 0.5d);
-            if (rowElement.getAbsoluteTop() + bottomBorder >= escalator
-                    .getHeader().getElement().getAbsoluteBottom()
-                    && rowElement.getAbsoluteBottom() <= escalator.getFooter()
-                            .getElement().getAbsoluteTop() + bottomBorder) {
-                whenVisible.run();
-                return;
-            }
+        boolean waitForCache = false;
+        if (getDataSource().getRow(rowIndex) == null) {
+            // not yet in cache, wait for this to change
+            waitForCache = true;
+
+            Reference<Registration> registration = new Reference<>();
+            registration.set(getDataSource()
+                    .addDataChangeHandler(new DataChangeHandler() {
+                        @Override
+                        public void resetDataAndSize(int estimatedNewDataSize) {
+                            // data set changed, cancel the operation
+                            registration.get().remove();
+                        }
+
+                        @Override
+                        public void dataUpdated(int firstRowIndex,
+                                int numberOfRows) {
+                            // NOP
+                        }
+
+                        @Override
+                        public void dataRemoved(int firstRowIndex,
+                                int numberOfRows) {
+                            // data set changed, cancel the operation
+                            registration.get().remove();
+                        }
+
+                        @Override
+                        public void dataAvailable(int firstRowIndex,
+                                int numberOfRows) {
+                            // if new available range contains the row,
+                            // try again
+                            if (Range.withLength(firstRowIndex, numberOfRows)
+                                    .contains(rowIndex)) {
+                                registration.get().remove();
+                                waitUntilVisible(rowIndex, destination,
+                                        whenVisible);
+                            }
+                        }
+
+                        @Override
+                        public void dataAdded(int firstRowIndex,
+                                int numberOfRows) {
+                            // data set changed, cancel the operation
+                            registration.get().remove();
+                        }
+                    }));
         }
 
-        Reference<HandlerRegistration> registration = new Reference<>();
-        registration.set(addScrollHandler(event -> {
-            if (escalator.getVisibleRowRange().contains(rowIndex)) {
-                registration.get().removeHandler();
-                whenVisible.run();
-            }
-        }));
         scrollToRow(rowIndex, destination);
+
+        if (!waitForCache) {
+            // all necessary adjustments done, time to perform
+            whenVisible.run();
+        }
     }
 
     /**