Fix selection column size calculation without data (#10384)

3 files changed, 145 insertions, 44 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 266bffd3f6..38cdfa4c3d 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -262,6 +262,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
              *            a plain text caption
              */
             public void setText(String text) {
+                detach();
                 this.content = text;
                 this.type = GridStaticCellType.TEXT;
                 section.requestSectionRefresh();
@@ -340,6 +341,8 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
              *            The html content of the cell
              */
             public void setHtml(String html) {
+                detach();
+
                 this.content = html;
                 this.type = GridStaticCellType.HTML;
                 section.requestSectionRefresh();
@@ -377,10 +380,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                     return;
                 }
 
-                if (this.content instanceof Widget) {
-                    // Old widget in the cell, detach it first
-                    section.getGrid().detachWidget((Widget) this.content);
-                }
+                detach();
                 this.content = widget;
                 this.type = GridStaticCellType.WIDGET;
                 section.requestSectionRefresh();
@@ -1030,9 +1030,9 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
              * refresh in the end.
              */
             Scheduler.get().scheduleFinally(() -> {
-                    if (markAsDirty) {
-                        markAsDirty = false;
-                        getGrid().refreshFooter();
+                if (markAsDirty) {
+                    markAsDirty = false;
+                    getGrid().refreshFooter();
                 }
             });
         }
@@ -1659,7 +1659,8 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 grid.refreshBody();
                 HeaderCell cell = grid.getDefaultHeaderRow()
                         .getCell(grid.selectionColumn);
-                if (cell.getType() == GridStaticCellType.WIDGET) { // if lazy provider, then no checkbox
+                // if lazy provider, then no checkbox
+                if (cell.getType() == GridStaticCellType.WIDGET) {
                     CheckBox checkBox = (CheckBox) grid.getDefaultHeaderRow()
                             .getCell(grid.selectionColumn).getWidget();
                     checkBox.setEnabled(isEnabled);
@@ -2913,7 +2914,8 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                 selectAllCheckBox = GWT.create(CheckBox.class);
                 selectAllCheckBox.setStylePrimaryName(
                         getStylePrimaryName() + SELECT_ALL_CHECKBOX_CLASSNAME);
-                // label of checkbox should only be visible for assistive devices
+                // label of checkbox should only be visible for assistive
+                // devices
                 selectAllCheckBox.addStyleName("v-assistive-device-only-label");
                 selectAllCheckBox.addValueChangeHandler(event -> {
                     selected = event.getValue();
@@ -3060,7 +3062,6 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             if (selectAllCheckBoxVisible) {
                 selectionCell.setWidget(selectAllCheckBox);
             } else {
-                selectAllCheckBox.removeFromParent();
                 selectionCell.setText("");
             }
         }
@@ -3307,12 +3308,27 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             assert !(currentDataAvailable.isEmpty() && dataSource
                     .isWaitingForData()) : "Trying to calculate column widths without data while data is still being fetched.";
 
+            // Make SelectAllCheckbox visible
+            getSelectionColumn().ifPresent(col -> {
+                HeaderCell headerCell = getDefaultHeaderRow().getCell(col);
+                if (headerCell.getType().equals(GridStaticCellType.WIDGET)) {
+                    // SelectAllCheckbox is present already
+                    return;
+                }
+                headerCell.setWidget(col.selectAllCheckBox);
+                refreshHeader(); // Paint.
+            });
+
             if (columnsAreGuaranteedToBeWiderThanGrid()) {
                 applyColumnWidths();
             } else {
                 applyColumnWidthsWithExpansion();
             }
 
+            // Hide the SelectAllCheckbox if needed
+            getSelectionColumn()
+                    .ifPresent(SelectionColumn::doSetSelectAllCheckBoxVisible);
+
             // Update latest width to prevent recalculate on height change.
             lastCalculatedInnerWidth = escalator.getInnerWidth();
         }
@@ -3842,11 +3858,10 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                             && event.getKeyCode() == KeyCodes.KEY_ENTER) {
                         final MenuItem item = getSelectedItem();
                         super.onBrowserEvent(event);
-                        Scheduler.get()
-                                .scheduleDeferred(() -> {
-                                    selectItem(item);
-                                    focus();
-                                });
+                        Scheduler.get().scheduleDeferred(() -> {
+                            selectItem(item);
+                            focus();
+                        });
 
                     } else {
                         super.onBrowserEvent(event);
@@ -4030,12 +4045,11 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         }
 
         private MenuItem createToggle(final Column<?, T> column) {
-            MenuItem toggle = new MenuItem(createHTML(column), true,
-                    () -> {
-                        hidingColumn = true;
-                        column.setHidden(!column.isHidden(), true);
-                        hidingColumn = false;
-                    });
+            MenuItem toggle = new MenuItem(createHTML(column), true, () -> {
+                hidingColumn = true;
+                column.setHidden(!column.isHidden(), true);
+                hidingColumn = false;
+            });
             toggle.addStyleName("column-hiding-toggle");
             columnToHidingToggleMap.put(column, toggle);
             return toggle;
@@ -4852,6 +4866,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
             return this;
         }
+
         /**
          * Returns the current header aria-label for this column.
          *
@@ -5989,13 +6004,15 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             final Column<?, T> column = getVisibleColumn(cell.getColumn());
 
             if (column.getAssistiveCaption() != null) {
-                cellElement.setAttribute("aria-label", column.getAssistiveCaption());
+                cellElement.setAttribute("aria-label",
+                        column.getAssistiveCaption());
             } else {
                 cellElement.removeAttribute("aria-label");
             }
         }
 
-        private void addSortingIndicatorsToHeaderRow(HeaderRow headerRow, FlyweightCell cell) {
+        private void addSortingIndicatorsToHeaderRow(HeaderRow headerRow,
+                FlyweightCell cell) {
 
             Element cellElement = cell.getElement();
 
@@ -6168,14 +6185,12 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
         escalator.addScrollHandler(event -> fireEvent(new ScrollEvent()));
 
-        escalator.addRowVisibilityChangeHandler(
-                event -> {
-                    if (dataSource != null && dataSource.size() != 0) {
-                        dataSource.ensureAvailability(
-                                event.getFirstVisibleRow(),
-                                event.getVisibleRowCount());
-                    }
-                });
+        escalator.addRowVisibilityChangeHandler(event -> {
+            if (dataSource != null && dataSource.size() != 0) {
+                dataSource.ensureAvailability(event.getFirstVisibleRow(),
+                        event.getVisibleRowCount());
+            }
+        });
 
         // Default action on SelectionEvents. Refresh the body so changed
         // become visible.
@@ -8893,21 +8908,21 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
          * Delay calculation to be deferred so Escalator can do it's magic.
          */
         Scheduler.get().scheduleFinally(() -> {
-                if (escalator
-                        .getInnerWidth() != autoColumnWidthsRecalculator.lastCalculatedInnerWidth) {
-                    recalculateColumnWidths();
-                }
+            if (escalator
+                    .getInnerWidth() != autoColumnWidthsRecalculator.lastCalculatedInnerWidth) {
+                recalculateColumnWidths();
+            }
 
-                // Vertical resizing could make editor positioning invalid so it
-                // needs to be recalculated on resize
-                if (isEditorActive()) {
-                    editor.updateVerticalScrollPosition();
-                }
+            // Vertical resizing could make editor positioning invalid so it
+            // needs to be recalculated on resize
+            if (isEditorActive()) {
+                editor.updateVerticalScrollPosition();
+            }
 
-                // if there is a resize, we need to refresh the body to avoid an
-                // off-by-one error which occurs when the user scrolls all the
-                // way to the bottom.
-                refreshBody();
+            // if there is a resize, we need to refresh the body to avoid an
+            // off-by-one error which occurs when the user scrolls all the
+            // way to the bottom.
+            refreshBody();
         });
     }
 
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridMultiSelectEmpty.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridMultiSelectEmpty.java
new file mode 100644
index 0000000000..7aa1c0f2fd
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridMultiSelectEmpty.java
@@ -0,0 +1,44 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.data.provider.DataProvider;
+import com.vaadin.data.provider.ListDataProvider;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.SelectionMode;
+import com.vaadin.ui.components.grid.MultiSelectionModel;
+import com.vaadin.ui.components.grid.MultiSelectionModel.SelectAllCheckBoxVisibility;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class GridMultiSelectEmpty extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Grid<String> grid = new Grid<>();
+        grid.addColumn(t -> t).setCaption("String");
+        grid.setSelectionMode(SelectionMode.MULTI);
+        MultiSelectionModel<String> selectionModel = (MultiSelectionModel<String>) grid
+                .getSelectionModel();
+        selectionModel.setSelectAllCheckBoxVisibility(
+                SelectAllCheckBoxVisibility.HIDDEN);
+
+        List<String> items = new ArrayList<>();
+        ListDataProvider<String> dataProvider = DataProvider
+                .ofCollection(items);
+        grid.setDataProvider(dataProvider);
+
+        addComponent(grid);
+        addComponent(new Button("Add Row", e -> {
+            items.add("Foo!");
+            dataProvider.refreshAll();
+        }));
+        addComponent(new Button("Recalculate", e -> {
+            grid.recalculateColumnWidths();
+        }));
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridMultiSelectEmptyTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridMultiSelectEmptyTest.java
new file mode 100644
index 0000000000..baf85cc45a
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridMultiSelectEmptyTest.java
@@ -0,0 +1,42 @@
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridMultiSelectEmptyTest extends MultiBrowserTest {
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        // On PhantomJS the result is more correct before recalculation.
+        return getBrowsersExcludingPhantomJS();
+    }
+
+    @Test
+    public void testCheckBoxColumnCorrectSize() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        int startingWidth = grid.getHeaderCell(0, 0).getSize().getWidth();
+        $(ButtonElement.class).caption("Add Row").first().click();
+        int currentWidth = grid.getHeaderCell(0, 0).getSize().getWidth();
+
+        assertEquals(
+                "Checkbox column size should not change when data is added",
+                startingWidth, currentWidth);
+
+        $(ButtonElement.class).caption("Recalculate").first().click();
+        currentWidth = grid.getHeaderCell(0, 0).getSize().getWidth();
+        assertEquals(
+                "Checkbox column size should not change when columns are recalculated",
+                startingWidth, currentWidth);
+    }
+
+}