Simplify Grid sidebar handling. (#11920)

There is no need to reset the sidebar for every column that is set
hidable within the same roundtrip. Because no other layouting depends on
the sidebar contents, we can safely delay the reset until all columns
have got their hidabiity status updated. As a bonus point, when the
reset itself is delayed, the headers have time to get populated and the
sidebar button's height can be determined immediately at attach. The
reset ensures that attach is called every time a column's hidability is
changed.

1 files changed, 40 insertions, 17 deletions

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 302ccb71b0..6a42d3708f 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -4104,15 +4104,25 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
         private void setHeightToHeaderCellHeight() {
             RowContainer header = grid.escalator.getHeader();
-            if (header.getRowCount() == 0
+            if (!WidgetUtil.isDisplayed(header.getElement())
+                    || header.getRowCount() == 0
                     || !header.getRowElement(0).hasChildNodes()) {
                 getLogger().info(
                         "No header cell available when calculating sidebar button height");
-                openCloseButton.setHeight(header.getDefaultRowHeight() + "px");
+                // If the Grid is hidden with styles when this is called the
+                // border height will be off, but it's usually only a matter of
+                // a pixel or so. Removing a style name cannot trigger a full
+                // refresh of the layout, it's developer's responsibility to do
+                // that where needed.
+                double height = header.getDefaultRowHeight()
+                        - WidgetUtil.measureVerticalBorder(getElement()) / 2;
+                openCloseButton.setHeight(height + "px");
 
                 return;
             }
 
+            // Use actual height instead of expected height in case the height
+            // is modified by styles.
             Element firstHeaderCell = header.getRowElement(0)
                     .getFirstChildElement();
             double height = WidgetUtil
@@ -4131,8 +4141,6 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 close();
                 grid.getElement().appendChild(getElement());
                 Grid.setParent(this, grid);
-                // border calculation won't work until attached
-                setHeightToHeaderCellHeight();
             }
         }
 
@@ -4143,10 +4151,9 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         @Override
         protected void onAttach() {
             super.onAttach();
-            // make sure the button will get correct height if the button should
-            // be visible when the grid is rendered the first time.
-            Scheduler.get()
-                    .scheduleDeferred(() -> setHeightToHeaderCellHeight());
+            // Make sure the button will get correct height whenever the Sidebar
+            // is added to the Grid.
+            setHeightToHeaderCellHeight();
         }
 
         @Override
@@ -4179,6 +4186,12 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
          */
         private boolean hidingColumn;
 
+        /**
+         * When several columns are set hidable, don't reset the Sidebar for
+         * every column separately.
+         */
+        private boolean toggleUpdateTriggered;
+
         private void updateColumnHidable(final Column<?, T> column) {
             if (column.isHidable()) {
                 MenuItem toggle = columnToHidingToggleMap.get(column);
@@ -4224,16 +4237,26 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         }
 
         private void updateTogglesOrder() {
-            if (!hidingColumn) {
-                int lastIndex = 0;
-                for (Column<?, T> column : getColumns()) {
-                    if (column.isHidable()) {
-                        final MenuItem menuItem = columnToHidingToggleMap
-                                .get(column);
-                        sidebar.menuBar.removeItem(menuItem);
-                        sidebar.menuBar.insertItem(menuItem, lastIndex++);
+            if (!hidingColumn && !toggleUpdateTriggered) {
+                // This method is called whenever a column is set hidable. If
+                // there are multiple hidable columns, it will get called
+                // separately for all of them. There is no need to update the
+                // order more than once and no other layouting is dependent on
+                // the Sidebar layouting getting finished first, so wait until
+                // all calls have arrived before proceeding further.
+                toggleUpdateTriggered = true;
+                Scheduler.get().scheduleFinally(() -> {
+                    int lastIndex = 0;
+                    for (Column<?, T> column : getColumns()) {
+                        if (column.isHidable()) {
+                            final MenuItem menuItem = columnToHidingToggleMap
+                                    .get(column);
+                            sidebar.menuBar.removeItem(menuItem);
+                            sidebar.menuBar.insertItem(menuItem, lastIndex++);
+                        }
                     }
-                }
+                    toggleUpdateTriggered = false;
+                });
             }
         }