Add column width recalculation when vertical scrollbar hidden/shown. (#12058)

- If the Grid has any columns with non-fixed widths, the presence of a
vertical scrollbar affects the column width calculations. Horizontal
scrollbar should only be shown when actually needed.

6 files changed, 160 insertions, 7 deletions

diff --git a/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java b/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java
new file mode 100644
index 0000000000..1d24fccd2e
--- /dev/null
+++ b/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright 2000-2018 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.client.widget.grid.events;
+
+import com.google.gwt.event.shared.EventHandler;
+import com.google.gwt.event.shared.GwtEvent;
+
+/**
+ * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+ * <p>
+ * Event handler that gets notified when the visibility of the vertical
+ * scrollbar of the Escalator changes.
+ *
+ * @author Vaadin Ltd
+ */
+public interface VerticalScrollbarVisibilityChangeHandler
+        extends EventHandler {
+
+    /**
+     * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+     * <p>
+     * Called when the visibility of the vertical scrollbar of the Escalator
+     * changes.
+     *
+     * @param event
+     *            the row visibility change event describing the change
+     */
+    void onVisibilityChange(
+            VerticalScrollbarVisibilityChangeEvent event);
+
+    /**
+     * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+     * <p>
+     * Event fired when the visibility of the vertical scrollbar of the
+     * Escalator changes.
+     *
+     * @author Vaadin Ltd
+     */
+    public class VerticalScrollbarVisibilityChangeEvent extends
+            GwtEvent<VerticalScrollbarVisibilityChangeHandler> {
+        /**
+         * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+         * <p>
+         * The type of this event.
+         */
+        public static final Type<VerticalScrollbarVisibilityChangeHandler> TYPE = new Type<>();
+
+        /**
+         * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+         * <p>
+         * Creates a new Escalator vertical scrollbar visibility change event.
+         *
+         */
+        public VerticalScrollbarVisibilityChangeEvent() {
+            // NOP
+        }
+
+        /*
+         * (non-Javadoc)
+         *
+         * @see com.google.gwt.event.shared.GwtEvent#getAssociatedType()
+         */
+        @Override
+        public Type<VerticalScrollbarVisibilityChangeHandler> getAssociatedType() {
+            return TYPE;
+        }
+
+        /*
+         * (non-Javadoc)
+         *
+         * @see
+         * com.google.gwt.event.shared.GwtEvent#dispatch(com.google.gwt.event.
+         * shared .EventHandler)
+         */
+        @Override
+        protected void dispatch(
+                VerticalScrollbarVisibilityChangeHandler handler) {
+            handler.onVisibilityChange(this);
+        }
+    }
+}
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 2eab5d99c4..7774682dad 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -98,6 +98,8 @@ import com.vaadin.client.widget.grid.events.EscalatorSizeChangeHandler;
 import com.vaadin.client.widget.grid.events.EscalatorSizeChangeHandler.EscalatorSizeChangeEvent;
 import com.vaadin.client.widget.grid.events.ScrollEvent;
 import com.vaadin.client.widget.grid.events.ScrollHandler;
+import com.vaadin.client.widget.grid.events.VerticalScrollbarVisibilityChangeHandler;
+import com.vaadin.client.widget.grid.events.VerticalScrollbarVisibilityChangeHandler.VerticalScrollbarVisibilityChangeEvent;
 import com.vaadin.client.widgets.Escalator.JsniUtil.TouchHandlerBundle;
 import com.vaadin.shared.Range;
 import com.vaadin.shared.ui.grid.HeightMode;
@@ -7291,6 +7293,29 @@ public class Escalator extends Widget
         root.appendChild(verticalScrollbar.getElement());
         verticalScrollbar.addScrollHandler(scrollHandler);
         verticalScrollbar.setScrollbarThickness(scrollbarThickness);
+        verticalScrollbar
+                .addVisibilityHandler(new ScrollbarBundle.VisibilityHandler() {
+
+                    private boolean queued = false;
+
+                    @Override
+                    public void visibilityChanged(
+                            ScrollbarBundle.VisibilityChangeEvent event) {
+                        if (queued) {
+                            return;
+                        }
+                        queued = true;
+
+                        /*
+                         * We either lost or gained a scrollbar. In either case,
+                         * we may need to update the column widths.
+                         */
+                        Scheduler.get().scheduleFinally(() -> {
+                            fireVerticalScrollbarVisibilityChangeEvent();
+                            queued = false;
+                        });
+                    }
+                });
 
         root.appendChild(horizontalScrollbar.getElement());
         horizontalScrollbar.addScrollHandler(scrollHandler);
@@ -7876,6 +7901,26 @@ public class Escalator extends Widget
     /**
      * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
      * <p>
+     * Adds an event handler that gets notified when the visibility of the
+     * vertical scrollbar changes.
+     *
+     * @param verticalScrollbarVisibilityChangeHandler
+     *            the event handler
+     * @return a handler registration for the added handler
+     */
+    public HandlerRegistration addVerticalScrollbarVisibilityChangeHandler(
+            VerticalScrollbarVisibilityChangeHandler verticalScrollbarVisibilityChangeHandler) {
+        return addHandler(verticalScrollbarVisibilityChangeHandler,
+                VerticalScrollbarVisibilityChangeEvent.TYPE);
+    }
+
+    private void fireVerticalScrollbarVisibilityChangeEvent() {
+        fireEvent(new VerticalScrollbarVisibilityChangeEvent());
+    }
+
+    /**
+     * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME!
+     * <p>
      * Adds an event handler that gets notified when the Escalator size changes.
      *
      * @param escalatorSizeChangeHandler
diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index 1226920bd1..996eae41be 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -4294,6 +4294,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      */
     private DataSource<T> dataSource;
     private Registration changeHandler;
+    private boolean recalculateColumnWidthsNeeded = false;
 
     /**
      * Currently available row range in DataSource.
@@ -5319,8 +5320,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 } else {
                     this.hidden = hidden;
 
-                    int columnIndex = grid.getVisibleColumns()
-                            .indexOf(this);
+                    int columnIndex = grid.getVisibleColumns().indexOf(this);
                     grid.escalator.getColumnConfiguration()
                             .insertColumns(columnIndex, 1);
 
@@ -6408,6 +6408,15 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             }
         });
 
+        escalator.addVerticalScrollbarVisibilityChangeHandler(event -> {
+            if (!(currentDataAvailable.isEmpty()
+                    && escalator.getBody().getRowCount() > 0)) {
+                recalculateColumnWidths();
+            } else {
+                recalculateColumnWidthsNeeded = true;
+            }
+        });
+
         // Default action on SelectionEvents. Refresh the body so changed
         // become visible.
         addSelectionHandler(new SelectionHandler<T>() {
@@ -7247,7 +7256,6 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         this.dataSource = dataSource;
         changeHandler = dataSource
                 .addDataChangeHandler(new DataChangeHandler() {
-                    private boolean recalculateColumnWidthsNeeded = false;
 
                     @Override
                     public void dataUpdated(int firstIndex, int numberOfItems) {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java
index b677cee903..3628c4adaf 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java
@@ -26,7 +26,7 @@ public class GridSizeChange extends AbstractTestUI {
     protected void setup(VaadinRequest request) {
         grid = new Grid<>();
         data = new ArrayList<>();
-        for (int i = 0; i < 10; ++i) {
+        for (int i = 0; i < 8; ++i) {
             data.add(i);
             ++counter;
         }
@@ -39,8 +39,8 @@ public class GridSizeChange extends AbstractTestUI {
 
         // set height mode and height
         grid.setHeightMode(HeightMode.ROW);
-        grid.setHeightByRows(10);
-        grid.setWidth(90, Unit.PIXELS);
+        grid.setHeightByRows(8);
+        grid.setWidth(100, Unit.PIXELS);
 
         // create a tabsheet with one tab and place grid inside
         VerticalLayout tab = new VerticalLayout();
diff --git a/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java b/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java
index 4fc4106bde..c259e34837 100644
--- a/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java
@@ -25,7 +25,7 @@ public class VerifyBrowserVersionTest extends MultiBrowserTest {
             // Chrome version does not necessarily match the desired version
             // because of auto updates...
             browserIdentifier = getExpectedUserAgentString(
-                    getDesiredCapabilities()) + "83";
+                    getDesiredCapabilities()) + "84";
         } else if (BrowserUtil.isFirefox(getDesiredCapabilities())) {
             browserIdentifier = getExpectedUserAgentString(
                     getDesiredCapabilities()) + "75";
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java
index 3b8e485ed9..48a56b5291 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java
@@ -41,23 +41,27 @@ public class GridSizeChangeTest extends MultiBrowserTest {
 
         assertGridWithinTabSheet();
         ensureVerticalScrollbar(true);
+        ensureHorizontalScrollbar(false);
 
         $(ButtonElement.class).caption("Remove row").first().click();
         // height matches rows -> no scrollbar
 
         assertGridWithinTabSheet();
         ensureVerticalScrollbar(false);
+        ensureHorizontalScrollbar(false);
 
         $(ButtonElement.class).caption("Reduce width").first().click();
         // column too wide -> scrollbar
 
         assertGridWithinTabSheet();
+        ensureVerticalScrollbar(false);
         ensureHorizontalScrollbar(true);
 
         $(ButtonElement.class).caption("Increase width").first().click();
         // column fits -> no scrollbar
 
         assertGridWithinTabSheet();
+        ensureVerticalScrollbar(false);
         ensureHorizontalScrollbar(false);
 
         $(ButtonElement.class).caption("Add row").first().click();
@@ -65,12 +69,14 @@ public class GridSizeChangeTest extends MultiBrowserTest {
 
         assertGridWithinTabSheet();
         ensureVerticalScrollbar(true);
+        ensureHorizontalScrollbar(false);
 
         $(ButtonElement.class).caption("Increase height").first().click();
         // height matches rows -> no scrollbar
 
         assertGridWithinTabSheet();
         ensureVerticalScrollbar(false);
+        ensureHorizontalScrollbar(false);
     }
 
     private void ensureVerticalScrollbar(boolean displayed) {