Fix Escalator onResize to layout Finally instead of Deferred (#18751)

This patch also corrects a possible issue with calculating columns when
it is not attached.

Change-Id: I616eb0f6d060991d9f461b7e2b1e3f7f30fbd122

4 files changed, 133 insertions, 17 deletions

diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index 881ac7d7ed..761edf1f11 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -375,13 +375,14 @@ public class Escalator extends Widget implements RequiresResize,
 
             // Duration of the inertial scrolling simulation. Devices with
             // larger screens take longer durations.
-            private static final int DURATION = (int)Window.getClientHeight();
+            private static final int DURATION = Window.getClientHeight();
             // multiply scroll velocity with repeated touching
             private int acceleration = 1;
             private boolean touching = false;
             // Two movement objects for storing status and processing touches
             private Movement yMov, xMov;
-            final double MIN_VEL = 0.6, MAX_VEL = 4, F_VEL = 1500, F_ACC = 0.7, F_AXIS = 1;
+            final double MIN_VEL = 0.6, MAX_VEL = 4, F_VEL = 1500, F_ACC = 0.7,
+                    F_AXIS = 1;
 
             // The object to deal with one direction scrolling
             private class Movement {
@@ -392,7 +393,8 @@ public class Escalator extends Widget implements RequiresResize,
 
                 public Movement(boolean vertical) {
                     this.vertical = vertical;
-                    scroll = vertical ? escalator.verticalScrollbar : escalator.horizontalScrollbar;
+                    scroll = vertical ? escalator.verticalScrollbar
+                            : escalator.horizontalScrollbar;
                 }
 
                 public void startTouch(CustomTouchEvent event) {
@@ -400,6 +402,7 @@ public class Escalator extends Widget implements RequiresResize,
                     prevPos = pagePosition(event);
                     prevTime = Duration.currentTimeMillis();
                 }
+
                 public void moveTouch(CustomTouchEvent event) {
                     double pagePosition = pagePosition(event);
                     if (pagePosition > -1) {
@@ -407,7 +410,8 @@ public class Escalator extends Widget implements RequiresResize,
                         double now = Duration.currentTimeMillis();
                         double ellapsed = now - prevTime;
                         velocity = delta / ellapsed;
-                        // if last speed was so low, reset speeds and start storing again
+                        // if last speed was so low, reset speeds and start
+                        // storing again
                         if (speeds.size() > 0 && !validSpeed(speeds.get(0))) {
                             speeds.clear();
                             run = true;
@@ -417,6 +421,7 @@ public class Escalator extends Widget implements RequiresResize,
                         prevPos = pagePosition;
                     }
                 }
+
                 public void endTouch(CustomTouchEvent event) {
                     // Compute average speed
                     velocity = 0;
@@ -424,20 +429,27 @@ public class Escalator extends Widget implements RequiresResize,
                         velocity += s / speeds.size();
                     }
                     position = scroll.getScrollPos();
-                    // Compute offset, and adjust it with an easing curve so as movement is smoother.
-                    offset = F_VEL * velocity * acceleration * easingInOutCos(velocity, MAX_VEL);
+
+                    // Compute offset, and adjust it with an easing curve so as
+                    // movement is smoother.
+                    offset = F_VEL * velocity * acceleration
+                            * easingInOutCos(velocity, MAX_VEL);
+
                     // Enable or disable inertia movement in this axis
                     run = validSpeed(velocity);
                     if (run) {
                         event.getNativeEvent().preventDefault();
                     }
                 }
+
                 void validate(Movement other) {
-                    if (!run || other.velocity > 0 && Math.abs(velocity / other.velocity) < F_AXIS) {
+                    if (!run || other.velocity > 0
+                            && Math.abs(velocity / other.velocity) < F_AXIS) {
                         delta = offset = 0;
                         run = false;
                     }
                 }
+
                 void stepAnimation(double progress) {
                     scroll.setScrollPos(position + offset * progress);
                 }
@@ -446,6 +458,7 @@ public class Escalator extends Widget implements RequiresResize,
                     JsArray<Touch> a = event.getNativeEvent().getTouches();
                     return vertical ? a.get(0).getPageY() : a.get(0).getPageX();
                 }
+
                 boolean validSpeed(double speed) {
                     return Math.abs(speed) > MIN_VEL;
                 }
@@ -453,17 +466,24 @@ public class Escalator extends Widget implements RequiresResize,
 
             // Using GWT animations which take care of native animation frames.
             private Animation animation = new Animation() {
+                @Override
                 public void onUpdate(double progress) {
                     xMov.stepAnimation(progress);
                     yMov.stepAnimation(progress);
                 }
+
+                @Override
                 public double interpolate(double progress) {
                     return easingOutCirc(progress);
                 };
+
+                @Override
                 public void onComplete() {
                     touching = false;
                     escalator.body.domSorter.reschedule();
                 };
+
+                @Override
                 public void run(int duration) {
                     if (xMov.run || yMov.run) {
                         super.run(duration);
@@ -503,7 +523,8 @@ public class Escalator extends Widget implements RequiresResize,
                     xMov.validate(yMov);
                     yMov.validate(xMov);
                     event.getNativeEvent().preventDefault();
-                    moveScrollFromEvent(escalator, xMov.delta, yMov.delta, event.getNativeEvent());
+                    moveScrollFromEvent(escalator, xMov.delta, yMov.delta,
+                            event.getNativeEvent());
                 }
             }
 
@@ -514,9 +535,10 @@ public class Escalator extends Widget implements RequiresResize,
                     xMov.validate(yMov);
                     yMov.validate(xMov);
                     // Adjust duration so as longer movements take more duration
-                    boolean vert = !xMov.run || yMov.run &&  Math.abs(yMov.offset) > Math.abs(xMov.offset);
+                    boolean vert = !xMov.run || yMov.run
+                            && Math.abs(yMov.offset) > Math.abs(xMov.offset);
                     double delta = Math.abs((vert ? yMov : xMov).offset);
-                    animation.run((int)(3 * DURATION * easingOutExp(delta)));
+                    animation.run((int) (3 * DURATION * easingOutExp(delta)));
                 }
             }
 
@@ -524,9 +546,11 @@ public class Escalator extends Widget implements RequiresResize,
                 return 0.5 - 0.5 * Math.cos(Math.PI * Math.signum(val)
                         * Math.min(Math.abs(val), max) / max);
             }
+
             private double easingOutExp(double delta) {
                 return (1 - Math.pow(2, -delta / 1000));
             }
+
             private double easingOutCirc(double progress) {
                 return Math.sqrt(1 - (progress - 1) * (progress - 1));
             }
@@ -558,7 +582,6 @@ public class Escalator extends Widget implements RequiresResize,
         }
     }
 
-
     /**
      * ScrollDestination case-specific handling logic.
      */
@@ -1323,7 +1346,8 @@ public class Escalator extends Widget implements RequiresResize,
                         cellElem.addClassName("frozen");
                         position.set(cellElem, scroller.lastScrollLeft, 0);
                     }
-                    if (columnConfiguration.frozenColumns > 0 && col == columnConfiguration.frozenColumns - 1) {
+                    if (columnConfiguration.frozenColumns > 0
+                            && col == columnConfiguration.frozenColumns - 1) {
                         cellElem.addClassName("last-frozen");
                     }
                 }
@@ -1590,7 +1614,8 @@ public class Escalator extends Widget implements RequiresResize,
             }
         }
 
-        private void toggleFrozenColumnClass(int column, boolean frozen, String className) {
+        private void toggleFrozenColumnClass(int column, boolean frozen,
+                String className) {
             final NodeList<TableRowElement> childRows = root.getRows();
 
             for (int row = 0; row < childRows.getLength(); row++) {
@@ -6411,7 +6436,7 @@ public class Escalator extends Widget implements RequiresResize,
     public void onResize() {
         if (isAttached() && !layoutIsScheduled) {
             layoutIsScheduled = true;
-            Scheduler.get().scheduleDeferred(layoutCommand);
+            Scheduler.get().scheduleFinally(layoutCommand);
         }
     }
 
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index f22f3ac506..b77299dc17 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -3093,7 +3093,6 @@ public class Grid<T> extends ResizeComposite implements
                 } else {
                     calculate();
                 }
-                lastCalculatedInnerWidth = escalator.getInnerWidth();
             }
         };
 
@@ -3115,7 +3114,7 @@ public class Grid<T> extends ResizeComposite implements
          * @see Column#setMaximumWidth(double)
          */
         public void schedule() {
-            if (!isScheduled) {
+            if (!isScheduled && isAttached()) {
                 isScheduled = true;
                 Scheduler.get().scheduleFinally(calculateCommand);
             }
@@ -3132,6 +3131,9 @@ public class Grid<T> extends ResizeComposite implements
             } else {
                 applyColumnWidthsWithExpansion();
             }
+
+            // Update latest width to prevent recalculate on height change.
+            lastCalculatedInnerWidth = escalator.getInnerWidth();
         }
 
         private boolean columnsAreGuaranteedToBeWiderThanGrid() {
@@ -8157,6 +8159,9 @@ public class Grid<T> extends ResizeComposite implements
         if (getEscalator().getBody().getRowCount() == 0 && dataSource != null) {
             setEscalatorSizeFromDataSource();
         }
+
+        // Grid was just attached to DOM. Column widths should be calculated.
+        recalculateColumnWidths();
     }
 
     @Override
@@ -8172,10 +8177,11 @@ public class Grid<T> extends ResizeComposite implements
     @Override
     public void onResize() {
         super.onResize();
+
         /*
          * Delay calculation to be deferred so Escalator can do it's magic.
          */
-        Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+        Scheduler.get().scheduleFinally(new ScheduledCommand() {
 
             @Override
             public void execute() {
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpand.java b/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpand.java
new file mode 100644
index 0000000000..5333b3698e
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpand.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.Column;
+import com.vaadin.ui.VerticalLayout;
+
+public class GridColumnAutoExpand extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final VerticalLayout layout = new VerticalLayout();
+        layout.setSizeFull();
+        layout.setMargin(true);
+        addComponent(layout);
+
+        Grid grid = new Grid("Broken Grid with Caption");
+        grid.setWidth("100%");
+        grid.setHeight("100px");
+
+        Column col1 = grid.addColumn("Col1");
+        col1.setWidth(100);
+
+        Column col2 = grid.addColumn("Col2");
+        col2.setMinimumWidth(100);
+        col2.setExpandRatio(1);
+
+        layout.addComponent(grid);
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpandTest.java b/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpandTest.java
new file mode 100644
index 0000000000..64c56e174e
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridColumnAutoExpandTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridColumnAutoExpandTest extends MultiBrowserTest {
+
+    @Test
+    public void testSecondColumnHasExpanded() {
+        openTestURL();
+
+        GridCellElement headerCell = $(GridElement.class).first()
+                .getHeaderCell(0, 1);
+
+        assertTrue("Column did not expand as expected", headerCell.getSize()
+                .getWidth() > 400);
+    }
+
+}