Improvements to ScrollDestination sanity checks (#11772)

- The new top row logical index should always be within the logical
range and high enough up to avoid leaving a gap if possible.
- Added regression testing for using the different scroll destination
types for scrolling to the top and to the bottom by index.

Fixes #11732

1 files changed, 26 insertions, 8 deletions

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 5c3bf593fb..e98ae2c578 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -5059,9 +5059,10 @@ public class Escalator extends Widget
                 break;
             case END:
                 // target row at the bottom of the viewport
-                newTopRowLogicalIndex = Math.min(
-                        lastVisibleIndexIfScrollingDown + 1, getRowCount() - 1)
+                newTopRowLogicalIndex = lastVisibleIndexIfScrollingDown + 1
                         - visualRangeLength + 1;
+                newTopRowLogicalIndex = ensureTopRowLogicalIndexSanity(
+                        newTopRowLogicalIndex);
                 if ((newTopRowLogicalIndex > oldTopRowLogicalIndex)
                         && (newTopRowLogicalIndex
                                 - oldTopRowLogicalIndex < visualRangeLength)) {
@@ -5078,10 +5079,8 @@ public class Escalator extends Widget
                 // target row at the middle of the viewport, padding has to be
                 // zero or we never would have reached this far
                 newTopRowLogicalIndex = targetRowIndex - visualRangeLength / 2;
-                // ensure we don't attempt to go beyond the bottom row
-                if (newTopRowLogicalIndex + visualRangeLength > getRowCount()) {
-                    newTopRowLogicalIndex = getRowCount() - visualRangeLength;
-                }
+                newTopRowLogicalIndex = ensureTopRowLogicalIndexSanity(
+                        newTopRowLogicalIndex);
                 if (newTopRowLogicalIndex < oldTopRowLogicalIndex) {
                     logicalTargetIndex = newTopRowLogicalIndex;
                 } else if (newTopRowLogicalIndex > oldTopRowLogicalIndex) {
@@ -5102,8 +5101,9 @@ public class Escalator extends Widget
             case START:
                 // target row at the top of the viewport, include buffer
                 // row if there is room for one
-                newTopRowLogicalIndex = Math
-                        .max(firstVisibleIndexIfScrollingUp - 1, 0);
+                newTopRowLogicalIndex = firstVisibleIndexIfScrollingUp - 1;
+                newTopRowLogicalIndex = ensureTopRowLogicalIndexSanity(
+                        newTopRowLogicalIndex);
                 if (getVisibleRowRange().contains(newTopRowLogicalIndex)) {
                     logicalTargetIndex = oldTopRowLogicalIndex
                             + visualRangeLength;
@@ -5141,6 +5141,24 @@ public class Escalator extends Widget
         }
 
         /**
+         * Modifies the proposed top row logical index to fit within the logical
+         * range and to not leave gaps if it is avoidable.
+         *
+         * @param proposedTopRowLogicalIndex
+         * @return an adjusted index, or the original if no changes were
+         *         necessary
+         */
+        private int ensureTopRowLogicalIndexSanity(
+                int proposedTopRowLogicalIndex) {
+            int newTopRowLogicalIndex = Math.max(proposedTopRowLogicalIndex, 0);
+            int visualRangeLength = visualRowOrder.size();
+            if (newTopRowLogicalIndex + visualRangeLength > getRowCount()) {
+                newTopRowLogicalIndex = getRowCount() - visualRangeLength;
+            }
+            return newTopRowLogicalIndex;
+        }
+
+        /**
          * Checks that scrolling is allowed and resets the scroll position if
          * it's not.
          *