From 83a1b8a0961cc9b2d43e01757530cefd035b0a22 Mon Sep 17 00:00:00 2001
From: Artur Signell <artur@vaadin.com>
Date: Fri, 2 Sep 2016 08:19:35 +0300
Subject: [PATCH] Update DOM and update escalator row count in the correct
 order (#19442)

If you show a column when Grid has no horizontal scrollbar, and is scrolled
all the way down, and the number of visible rows is slightly more than N then
Escalator will adjust the row count when the scrollbar is shown so that N-1
rows are visible. During this operation, the DOM must be updated for the new
column.

Change-Id: I0b6c845f96a57be1d64ef4e735aa2f77efbe589a
---
 .../com/vaadin/client/widgets/Escalator.java  | 13 ++++---
 .../server/GridColumnVisibilityTest.java      | 38 +++++++++++++++++--
 2 files changed, 43 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 52f981fe63..d867907996 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -4166,6 +4166,11 @@ public class Escalator extends Widget implements RequiresResize,
                 frozenColumns += numberOfColumns;
             }
 
+            // Add to DOM
+            header.paintInsertColumns(index, numberOfColumns, frozen);
+            body.paintInsertColumns(index, numberOfColumns, frozen);
+            footer.paintInsertColumns(index, numberOfColumns, frozen);
+
             // this needs to be before the scrollbar adjustment.
             boolean scrollbarWasNeeded = horizontalScrollbar.getOffsetSize() < horizontalScrollbar
                     .getScrollSize();
@@ -4173,14 +4178,12 @@ public class Escalator extends Widget implements RequiresResize,
             boolean scrollbarIsNowNeeded = horizontalScrollbar.getOffsetSize() < horizontalScrollbar
                     .getScrollSize();
             if (!scrollbarWasNeeded && scrollbarIsNowNeeded) {
+                // This might as a side effect move rows around (when scrolled
+                // all the way down) and require the DOM to be up to date, i.e.
+                // the column to be added
                 body.verifyEscalatorCount();
             }
 
-            // Add to DOM
-            header.paintInsertColumns(index, numberOfColumns, frozen);
-            body.paintInsertColumns(index, numberOfColumns, frozen);
-            footer.paintInsertColumns(index, numberOfColumns, frozen);
-
             // fix initial width
             if (header.getRowCount() > 0 || body.getRowCount() > 0
                     || footer.getRowCount() > 0) {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
index 6e0d7cc87a..11a6dcf4b3 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
@@ -1,12 +1,12 @@
 /*
  * 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
@@ -21,9 +21,11 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.vaadin.testbench.elements.GridElement;
 import com.vaadin.testbench.parallel.TestCategory;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
@@ -295,6 +297,33 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
         verifyColumnNotFrozen(2);
     }
 
+    @Test
+    public void showColumnAndScrollbarWhenScrolledDownAndVisibleRowsChange() throws Exception {
+        // Set a (un)suitable height
+        selectMenuPath("Component", "Size", "HeightMode Row");
+        selectMenuPath("Component", "Size", "Height by Rows", "4.33 rows");
+
+        toggleAllColumnsHidable();
+
+        // Hide all but the first 3
+        getSidebarOpenButton().click();
+        for (int i=3; i < 12; i++) {
+            getColumnHidingToggle(i).click();
+        }
+
+        getSidebarOpenButton().click();
+
+        // Scroll all the way to the end
+        $(GridElement.class).first().scrollToRow(999);
+
+        // Show the fourth column
+        getSidebarOpenButton().click();
+        getColumnHidingToggle(3).click();
+
+        // Make sure that the new column contains the data it should
+        Assert.assertEquals("(999, 3)", getGridElement().getCell(999, 3).getText());
+    }
+
     private void verifyColumnFrozen(int index) {
         assertTrue(getGridElement().getHeaderCell(0, index).isFrozen());
     }
@@ -306,6 +335,9 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
     private void toggleColumnHidable(int index) {
         selectMenuPath("Component", "Columns", "Column " + index, "Hidable");
     }
+    private void toggleAllColumnsHidable() {
+        selectMenuPath("Component", "Columns", "All columns hidable");
+    }
 
     private void addRemoveColumn(int index) {
         selectMenuPath("Component", "Columns", "Column " + index,
-- 
2.39.5