Escalator spacers are sized and placed like "fixed" (#16644)

The width of a spacer is now 100% of the viewport (width of escalator
minus possible scrollbars), and and are always horizontally frozen
with the viewport.

Change-Id: I6616747784cdb61551e144d941526dee815a7ef9

3 files changed, 184 insertions, 46 deletions

diff --git a/client/src/com/vaadin/client/widget/escalator/Spacer.java b/client/src/com/vaadin/client/widget/escalator/Spacer.java
index 371d539f57..1e9985d6c1 100644
--- a/client/src/com/vaadin/client/widget/escalator/Spacer.java
+++ b/client/src/com/vaadin/client/widget/escalator/Spacer.java
@@ -16,10 +16,10 @@
 package com.vaadin.client.widget.escalator;
 
 import com.google.gwt.dom.client.Element;
-import com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer;
 
 /**
- * A representation of a spacer element in a {@link BodyRowContainer}.
+ * A representation of a spacer element in a
+ * {@link com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer}.
  * 
  * @since
  * @author Vaadin Ltd
diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index 9d5526b14a..1d7d9a9910 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -1698,13 +1698,11 @@ public class Escalator extends Widget implements RequiresResize,
         public void setColumnFrozen(int column, boolean frozen) {
             final NodeList<TableRowElement> childRows = root.getRows();
 
-            if (column == 0 && this instanceof BodyRowContainer) {
-                // TODO
-                getLogger().warning("[[spacers]] freezing columns");
-            }
-
             for (int row = 0; row < childRows.getLength(); row++) {
                 final TableRowElement tr = childRows.getItem(row);
+                if (!rowCanBeFrozen(tr)) {
+                    continue;
+                }
 
                 TableCellElement cell = tr.getCells().getItem(column);
                 if (frozen) {
@@ -1723,20 +1721,32 @@ public class Escalator extends Widget implements RequiresResize,
         public void updateFreezePosition(int column, double scrollLeft) {
             final NodeList<TableRowElement> childRows = root.getRows();
 
-            if (column == 0 && this instanceof BodyRowContainer) {
-                // TODO
-                getLogger().warning("[[spacers]] update freeze position");
-            }
-
             for (int row = 0; row < childRows.getLength(); row++) {
                 final TableRowElement tr = childRows.getItem(row);
 
-                TableCellElement cell = tr.getCells().getItem(column);
-                position.set(cell, scrollLeft, 0);
+                if (rowCanBeFrozen(tr)) {
+                    TableCellElement cell = tr.getCells().getItem(column);
+                    position.set(cell, scrollLeft, 0);
+                }
             }
         }
 
         /**
+         * Checks whether a row is an element, or contains such elements, that
+         * can be frozen.
+         * <p>
+         * In practice, this applies for all header and footer rows. For body
+         * rows, it applies for all rows except spacer rows.
+         * 
+         * @param tr
+         *            the row element to check for if it is or has elements that
+         *            can be frozen
+         * @return <code>true</code> iff this the given element, or any of its
+         *         descendants, can be frozen
+         */
+        abstract protected boolean rowCanBeFrozen(TableRowElement tr);
+
+        /**
          * Iterates through all the cells in a column and returns the width of
          * the widest element in this RowContainer.
          * 
@@ -2231,6 +2241,12 @@ public class Escalator extends Widget implements RequiresResize,
         protected void paintInsertRows(int visualIndex, int numberOfRows) {
             paintInsertStaticRows(visualIndex, numberOfRows);
         }
+
+        @Override
+        protected boolean rowCanBeFrozen(TableRowElement tr) {
+            assert root.isOrHasChild(tr) : "Row does not belong to this table section";
+            return true;
+        }
     }
 
     private class HeaderRowContainer extends AbstractStaticRowContainer {
@@ -3858,6 +3874,15 @@ public class Escalator extends Widget implements RequiresResize,
             return root.getChildCount()
                     - spacerContainer.getSpacersInDom().size();
         }
+
+        @Override
+        protected boolean rowCanBeFrozen(TableRowElement tr) {
+            return visualRowOrder.contains(tr);
+        }
+
+        public void reapplySpacerWidths() {
+            spacerContainer.reapplySpacerWidths();
+        }
     }
 
     private class ColumnConfigurationImpl implements ColumnConfiguration {
@@ -4376,18 +4401,13 @@ public class Escalator extends Widget implements RequiresResize,
                 setPosition(getLeft() + x, getTop() + y);
             }
 
-            public double getLeft() {
-                // not implemented yet.
-                return 0;
-            }
-
             public void setupDom(double height) {
                 assert !domHasBeenSetup : "DOM can't be set up twice.";
                 assert RootPanel.get().getElement().isOrHasChild(root) : "Root element should've been attached to the DOM by now.";
                 domHasBeenSetup = true;
 
+                getRootElement().getStyle().setWidth(getInnerWidth(), Unit.PX);
                 setHeight(height);
-                root.getStyle().setWidth(100, Unit.PCT);
 
                 spacerElement.getStyle().setWidth(100, Unit.PCT);
                 spacerElement.setColSpan(getColumnConfiguration()
@@ -4508,6 +4528,10 @@ public class Escalator extends Widget implements RequiresResize,
                 return positions.getTop(getRootElement());
             }
 
+            public double getLeft() {
+                return positions.getLeft(getRootElement());
+            }
+
             /**
              * Sets a new row index for this spacer. Also updates the bookeeping
              * at {@link SpacerContainer#rowIndexToSpacer}.
@@ -4526,6 +4550,23 @@ public class Escalator extends Widget implements RequiresResize,
 
         private SpacerUpdater spacerUpdater = SpacerUpdater.NULL;
 
+        private final ScrollHandler spacerScroller = new ScrollHandler() {
+            private double prevScrollX = 0;
+
+            @Override
+            public void onScroll(ScrollEvent event) {
+                if (WidgetUtil.pixelValuesEqual(getScrollLeft(), prevScrollX)) {
+                    return;
+                }
+
+                prevScrollX = getScrollLeft();
+                for (SpacerImpl spacer : rowIndexToSpacer.values()) {
+                    spacer.setPosition(prevScrollX, spacer.getTop());
+                }
+            }
+        };
+        private HandlerRegistration spacerScrollerRegistration;
+
         public void setSpacer(int rowIndex, double height)
                 throws IllegalArgumentException {
 
@@ -4546,6 +4587,13 @@ public class Escalator extends Widget implements RequiresResize,
             }
         }
 
+        public void reapplySpacerWidths() {
+            for (SpacerImpl spacer : rowIndexToSpacer.values()) {
+                spacer.getRootElement().getStyle()
+                        .setWidth(getInnerWidth(), Unit.PX);
+            }
+        }
+
         public void paintRemoveSpacers(Range removedRowsRange) {
             removeSpacers(removedRowsRange);
             shiftSpacersByRows(removedRowsRange.getStart(),
@@ -4559,6 +4607,10 @@ public class Escalator extends Widget implements RequiresResize,
                     .subMap(removedRange.getStart(), true,
                             removedRange.getEnd(), false);
 
+            if (removedSpacers.isEmpty()) {
+                return;
+            }
+
             for (SpacerImpl spacer : removedSpacers.values()) {
                 /*
                  * [[optimization]] TODO: Each invocation of the setHeight
@@ -4572,6 +4624,12 @@ public class Escalator extends Widget implements RequiresResize,
             }
 
             removedSpacers.clear();
+
+            if (rowIndexToSpacer.isEmpty()) {
+                assert spacerScrollerRegistration != null : "Spacer scroller registration was null";
+                spacerScrollerRegistration.removeHandler();
+                spacerScrollerRegistration = null;
+            }
         }
 
         /**
@@ -4846,10 +4904,14 @@ public class Escalator extends Widget implements RequiresResize,
         @SuppressWarnings("boxing")
         private void insertNewSpacer(int rowIndex, double height) {
 
+            if (spacerScrollerRegistration == null) {
+                spacerScrollerRegistration = addScrollHandler(spacerScroller);
+            }
+
             SpacerImpl spacer = new SpacerImpl(rowIndex);
 
             rowIndexToSpacer.put(rowIndex, spacer);
-            spacer.setPosition(0, calculateSpacerTop(rowIndex));
+            spacer.setPosition(getScrollLeft(), calculateSpacerTop(rowIndex));
 
             TableRowElement spacerRoot = spacer.getRootElement();
             spacerRoot.getStyle().setWidth(
@@ -4949,7 +5011,7 @@ public class Escalator extends Widget implements RequiresResize,
                 double diffPx) {
             for (SpacerImpl spacer : rowIndexToSpacer.tailMap(changedRowIndex,
                     false).values()) {
-                spacer.setPosition(0, spacer.getTop() + diffPx);
+                spacer.setPositionDiff(0, diffPx);
             }
         }
 
@@ -4982,11 +5044,13 @@ public class Escalator extends Widget implements RequiresResize,
          * A map containing cached values of an element's current top position.
          */
         private final Map<Element, Double> elementTopPositionMap = new HashMap<Element, Double>();
+        private final Map<Element, Double> elementLeftPositionMap = new HashMap<Element, Double>();
 
         public void set(final Element e, final double x, final double y) {
             assert e != null : "Element was null";
             position.set(e, x, y);
             elementTopPositionMap.put(e, Double.valueOf(y));
+            elementLeftPositionMap.put(e, Double.valueOf(x));
         }
 
         public double getTop(final Element e) {
@@ -4998,8 +5062,18 @@ public class Escalator extends Widget implements RequiresResize,
             return top.doubleValue();
         }
 
+        public double getLeft(final Element e) {
+            Double left = elementLeftPositionMap.get(e);
+            if (left == null) {
+                throw new IllegalArgumentException("Element " + e
+                        + " was not found in the position bookkeeping");
+            }
+            return left.doubleValue();
+        }
+
         public void remove(Element e) {
             elementTopPositionMap.remove(e);
+            elementLeftPositionMap.remove(e);
         }
     }
 
@@ -5621,6 +5695,7 @@ public class Escalator extends Widget implements RequiresResize,
 
         scroller.recalculateScrollbarsForVirtualViewport();
         body.verifyEscalatorCount();
+        body.reapplySpacerWidths();
         Profiler.leave("Escalator.recalculateElementSizes");
     }
 
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 85acdc29cc..5e56d9433a 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
@@ -44,7 +44,15 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
     private final static String TRANSLATE_VALUE_REGEX = 
             "translate(?:3d|)" // "translate" or "translate3d"
             + "\\(" // literal "("
-                + ".+?, " // the x argument, uninteresting
+                + "(" // start capturing the x argument
+                    + "[0-9]+" // the integer part of the value
+                    + "(?:" // start of the subpixel part of the value
+                        + "\\.[0-9]" // if we have a period, there must be at least one number after it
+                        + "[0-9]*" // any amount of accuracy afterwards is fine
+                    + ")?" // the subpixel part is optional
+                + ")"
+            + "(?:px)?" // we don't care if the values are suffixed by "px" or not.
+            + ", "
                 + "(" // start capturing the y argument
                     + "[0-9]+" // the integer part of the value
                     + "(?:" // start of the subpixel part of the value
@@ -52,14 +60,14 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
                         + "[0-9]*" // any amount of accuracy afterwards is fine
                     + ")?" // the subpixel part is optional
                 + ")"
-                + "(?:px)?" // we don't care if the values are suffixed by "px" or not.
-                + "(?:, .*?)?" // the possible z argument, uninteresting (translate doesn't have one, translate3d does)
-                + "\\)" // literal ")"
-                + ";?"; // optional ending semicolon
+            + "(?:px)?" // we don't care if the values are suffixed by "px" or not.
+            + "(?:, .*?)?" // the possible z argument, uninteresting (translate doesn't have one, translate3d does)
+            + "\\)" // literal ")"
+            + ";?"; // optional ending semicolon
     
     // 40px;
     // 12.34px
-    private final static String TOP_VALUE_REGEX =
+    private final static String PIXEL_VALUE_REGEX =
             "(" // capture the pixel value
                 + "[0-9]+" // the pixel argument
                 + "(?:" // start of the subpixel part of the value
@@ -75,11 +83,13 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
             .compile("transform: (.*?);"); // also matches "-webkit-transform";
     private final static Pattern TOP_CSS_PATTERN = Pattern
             .compile("top: (.*?);");
+    private final static Pattern LEFT_CSS_PATTERN = Pattern
+            .compile("left: (.*?);");
 
     private final static Pattern TRANSLATE_VALUE_PATTERN = Pattern
             .compile(TRANSLATE_VALUE_REGEX);
-    private final static Pattern TOP_VALUE_PATTERN = Pattern
-            .compile(TOP_VALUE_REGEX);
+    private final static Pattern PIXEL_VALUE_PATTERN = Pattern
+            .compile(PIXEL_VALUE_REGEX);
 
     @Before
     public void before() {
@@ -213,7 +223,33 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
 
     }
 
-    private static double getElementTop(WebElement element) {
+   @Test
+    public void spacersAreFixedInViewport_firstFreezeThenScroll() {
+        selectMenuPath(FEATURES, FROZEN_COLUMNS, FREEZE_1_COLUMN);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+        assertEquals("Spacer's left position should've been 0 at the "
+                + "beginning", 0d, getElementLeft(getSpacer(1)),
+                WidgetUtil.PIXEL_EPSILON);
+
+        int scrollTo = 10;
+        scrollHorizontallyTo(scrollTo);
+        assertEquals("Spacer's left position should've been " + scrollTo
+                + " after scrolling " + scrollTo + "px", scrollTo,
+                getElementLeft(getSpacer(1)), WidgetUtil.PIXEL_EPSILON);
+    }
+
+    @Test
+    public void spacersAreFixedInViewport_firstScrollThenFreeze() {
+        selectMenuPath(FEATURES, FROZEN_COLUMNS, FREEZE_1_COLUMN);
+        int scrollTo = 10;
+        scrollHorizontallyTo(scrollTo);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+        assertEquals("Spacer's left position should've been " + scrollTo
+                + " after scrolling " + scrollTo + "px", scrollTo,
+                getElementLeft(getSpacer(1)), WidgetUtil.PIXEL_EPSILON);
+    }
+
+    private static double[] getElementDimensions(WebElement element) {
         /*
          * we need to parse the style attribute, since using getCssValue gets a
          * normalized value that is harder to parse.
@@ -222,17 +258,34 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
 
         String transform = getTransformFromStyle(style);
         if (transform != null) {
-            return getTranslateYValue(transform);
+            return getTranslateValues(transform);
         }
 
+        double[] result = new double[] { -1, -1 };
+        String left = getLeftFromStyle(style);
+        if (left != null) {
+            result[1] = getPixelValue(left);
+        }
         String top = getTopFromStyle(style);
         if (top != null) {
-            return getTopValue(top);
+            result[0] = getPixelValue(top);
+        }
+
+        if (result[0] != -1 && result[1] != -1) {
+            return result;
+        } else {
+            throw new IllegalArgumentException(
+                    "Could not parse the top position from the CSS \"" + style
+                            + "\"");
         }
+    }
 
-        throw new IllegalArgumentException(
-                "Could not parse the top position from the CSS \"" + style
-                        + "\"");
+    private static double getElementTop(WebElement element) {
+        return getElementDimensions(element)[1];
+    }
+
+    private static double getElementLeft(WebElement element) {
+        return getElementDimensions(element)[0];
     }
 
     private static String getTransformFromStyle(String style) {
@@ -243,6 +296,10 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
         return getFromStyle(TOP_CSS_PATTERN, style);
     }
 
+    private static String getLeftFromStyle(String style) {
+        return getFromStyle(LEFT_CSS_PATTERN, style);
+    }
+
     private static String getFromStyle(Pattern pattern, String style) {
         Matcher matcher = pattern.matcher(style);
         if (matcher.find()) {
@@ -254,19 +311,25 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
         }
     }
 
-    private static double getTranslateYValue(String translate) {
-        return getValueFromCss(TRANSLATE_VALUE_PATTERN, translate);
-    }
+    /**
+     * @return {@code [0] == x}, {@code [1] == y}
+     */
+    private static double[] getTranslateValues(String translate) {
+        Matcher matcher = TRANSLATE_VALUE_PATTERN.matcher(translate);
+        assertTrue("no matches for " + translate + " against "
+                + TRANSLATE_VALUE_PATTERN, matcher.find());
+        assertEquals("wrong amout of groups matched in " + translate, 2,
+                matcher.groupCount());
 
-    private static double getTopValue(String top) {
-        return getValueFromCss(TOP_VALUE_PATTERN, top);
+        return new double[] { Double.parseDouble(matcher.group(1)),
+                Double.parseDouble(matcher.group(2)) };
     }
 
-    private static double getValueFromCss(Pattern pattern, String css) {
-        Matcher matcher = pattern.matcher(css);
-        assertTrue("no matches for " + css + " against "
-                + TRANSLATE_VALUE_PATTERN, matcher.find());
-        assertEquals("wrong amount of groups matched in " + css, 1,
+    private static double getPixelValue(String top) {
+        Matcher matcher = PIXEL_VALUE_PATTERN.matcher(top);
+        assertTrue("no matches for " + top + " against " + PIXEL_VALUE_PATTERN,
+                matcher.find());
+        assertEquals("wrong amount of groups matched in " + top, 1,
                 matcher.groupCount());
         return Double.parseDouble(matcher.group(1));
     }