Escalator tries to hide out-of-sight spacers from the user (#17353)

Change-Id: I807896e3cdd07f4a43a265ba720435f01778f7e1

3 files changed, 120 insertions, 4 deletions

diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index fe8ba4f67e..01567143dd 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -3720,19 +3720,32 @@ public class Escalator extends Widget implements RequiresResize,
 
             List<TableRowElement> orderedBodyRows = new ArrayList<TableRowElement>(
                     visualRowOrder);
-            for (int i = 0; i < visualRowOrder.size(); i++) {
-                SpacerContainer.SpacerImpl spacer = body.spacerContainer
-                        .getSpacer(getTopRowLogicalIndex() + i);
+            Map<Integer, SpacerContainer.SpacerImpl> spacers = body.spacerContainer
+                    .getSpacers();
+
+            /*
+             * Start at -1 to include a spacer that is rendered above the
+             * viewport, but its parent row is still not shown
+             */
+            for (int i = -1; i < visualRowOrder.size(); i++) {
+                SpacerContainer.SpacerImpl spacer = spacers.remove(Integer
+                        .valueOf(getTopRowLogicalIndex() + i));
 
                 if (spacer != null) {
                     orderedBodyRows.add(i + 1, spacer.getRootElement());
+                    spacer.show();
                 }
             }
             /*
              * At this point, invisible spacers aren't reordered, so their
-             * position in the DOM is undefined.
+             * position in the DOM will remain undefined.
              */
 
+            // If a spacer was not reordered, it means that it's out of view.
+            for (SpacerContainer.SpacerImpl unmovedSpacer : spacers.values()) {
+                unmovedSpacer.hide();
+            }
+
             /*
              * If we have a focused row, start in the mode where we put
              * everything underneath that row. Otherwise, all rows are placed as
@@ -4755,6 +4768,33 @@ public class Escalator extends Widget implements RequiresResize,
                 root.setPropertyInt(SPACER_LOGICAL_ROW_PROPERTY, rowIndex);
                 rowIndexToSpacer.put(this.rowIndex, this);
             }
+
+            /**
+             * Updates the spacer's visibility parameters, based on whether it
+             * is being currently visible or not.
+             */
+            public void updateVisibility() {
+                if (isInViewport()) {
+                    show();
+                } else {
+                    hide();
+                }
+            }
+
+            private boolean isInViewport() {
+                int top = (int) Math.ceil(getTop());
+                int height = (int) Math.floor(getHeight());
+                Range location = Range.withLength(top, height);
+                return getViewportPixels().intersects(location);
+            }
+
+            public void show() {
+                getRootElement().getStyle().clearDisplay();
+            }
+
+            public void hide() {
+                getRootElement().getStyle().setDisplay(Display.NONE);
+            }
         }
 
         private final TreeMap<Integer, SpacerImpl> rowIndexToSpacer = new TreeMap<Integer, SpacerImpl>();
@@ -4887,6 +4927,10 @@ public class Escalator extends Widget implements RequiresResize,
             }
         }
 
+        public Map<Integer, SpacerImpl> getSpacers() {
+            return new HashMap<Integer, SpacerImpl>(rowIndexToSpacer);
+        }
+
         /**
          * Calculates the sum of all spacers.
          * 
@@ -5239,6 +5283,8 @@ public class Escalator extends Widget implements RequiresResize,
             spacerUpdater.init(spacer);
             assert getElement().isOrHasChild(spacer.getRootElement()) : "Spacer's root element somehow got detached from Escalator during attaching";
             assert getElement().isOrHasChild(spacer.getElement()) : "Spacer element somehow got detached from Escalator during attaching";
+
+            spacer.updateVisibility();
         }
 
         public String getSubPartName(Element subElement) {
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
index e1a3fc0f55..862e959ebc 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
@@ -83,6 +83,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     protected static final String REMOVE = "Remove";
 
     protected static final String ROW_MINUS1 = "Row -1";
+    protected static final String ROW_0 = "Row 0";
     protected static final String ROW_1 = "Row 1";
     protected static final String ROW_25 = "Row 25";
     protected static final String ROW_50 = "Row 50";
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
index 7905906536..71cc19ecdd 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
@@ -17,6 +17,7 @@ package com.vaadin.tests.components.grid.basicfeatures.escalator;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -28,6 +29,7 @@ import org.junit.Before;
 import org.junit.ComparisonFailure;
 import org.junit.Test;
 import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.client.WidgetUtil;
@@ -423,6 +425,73 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
                 + "\")", cssClass.contains("-spacer"));
     }
 
+    @Test
+    public void spacerScrolledIntoViewGetsFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SET_100PX);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SCROLL_HERE_ANY_0PADDING);
+
+        tryToTabIntoFocusUpdaterElement();
+        assertEquals("input", getFocusedElement().getTagName());
+    }
+
+    @Test
+    public void spacerScrolledOutOfViewDoesNotGetFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SCROLL_HERE_ANY_0PADDING);
+
+        tryToTabIntoFocusUpdaterElement();
+        assertNotEquals("input", getFocusedElement().getTagName());
+    }
+
+    @Test
+    public void spacerOpenedInViewGetsFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+        tryToTabIntoFocusUpdaterElement();
+        assertEquals("input", getFocusedElement().getTagName());
+    }
+
+    @Test
+    public void spacerOpenedOutOfViewDoesNotGetFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SET_100PX);
+
+        tryToTabIntoFocusUpdaterElement();
+        assertNotEquals("input", getFocusedElement().getTagName());
+    }
+
+    @Test
+    public void spacerOpenedInViewAndScrolledOutAndBackAgainGetsFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_50);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SCROLL_HERE_ANY_0PADDING);
+
+        tryToTabIntoFocusUpdaterElement();
+        assertEquals("input", getFocusedElement().getTagName());
+    }
+
+    @Test
+    public void spacerOpenedOutOfViewAndScrolledInAndBackAgainDoesNotGetFocus() {
+        selectMenuPath(FEATURES, SPACERS, FOCUSABLE_UPDATER);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SET_100PX);
+        selectMenuPath(FEATURES, SPACERS, ROW_50, SCROLL_HERE_ANY_0PADDING);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_0);
+
+        tryToTabIntoFocusUpdaterElement();
+        assertNotEquals("input", getFocusedElement().getTagName());
+    }
+
+    private void tryToTabIntoFocusUpdaterElement() {
+        getEscalator().sendKeys( //
+                Keys.TAB, // v-ui v-scrollable
+                Keys.TAB, // menubar
+                Keys.TAB // <input>
+                );
+    }
+
     private WebElement getChild(WebElement parent, int childIndex) {
         return (WebElement) executeScript("return arguments[0].children["
                 + childIndex + "];", parent);