Fix to compatibility Grid sorting after removing multi-select. (#12012)

Adapted from V7 fix #10999

3 files changed, 108 insertions, 17 deletions

diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
index 4a7265a525..abfe761d4a 100755
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
@@ -2940,6 +2940,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
         private CheckBox selectAllCheckBox;
         private boolean userSelectionAllowed = true;
         private boolean enabled = true;
+        private HandlerRegistration headerClickHandler;
 
         SelectionColumn(final Renderer<Boolean> selectColumnRenderer) {
             super(selectColumnRenderer);
@@ -2993,24 +2994,28 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                         });
                 selectAllCheckBox.setValue(selected);
 
-                addHeaderClickHandler(new HeaderClickHandler() {
-                    @Override
-                    public void onClick(GridClickEvent event) {
-                        if (!userSelectionAllowed) {
-                            return;
-                        }
-
-                        CellReference<?> targetCell = event.getTargetCell();
-                        int defaultRowIndex = getHeader().getRows()
-                                .indexOf(getDefaultHeaderRow());
+                headerClickHandler = addHeaderClickHandler(
+                        new HeaderClickHandler() {
+                            @Override
+                            public void onClick(GridClickEvent event) {
+                                if (!userSelectionAllowed) {
+                                    return;
+                                }
 
-                        if (targetCell.getColumnIndex() == 0 && targetCell
-                                .getRowIndex() == defaultRowIndex) {
-                            selectAllCheckBox.setValue(
-                                    !selectAllCheckBox.getValue(), true);
-                        }
-                    }
-                });
+                                CellReference<?> targetCell = event
+                                        .getTargetCell();
+                                int defaultRowIndex = getHeader().getRows()
+                                        .indexOf(getDefaultHeaderRow());
+
+                                if (targetCell.getColumnIndex() == 0
+                                        && targetCell
+                                                .getRowIndex() == defaultRowIndex) {
+                                    selectAllCheckBox.setValue(
+                                            !selectAllCheckBox.getValue(),
+                                            true);
+                                }
+                            }
+                        });
 
                 // Select all with space when "select all" cell is active
                 addHeaderKeyUpHandler(new HeaderKeyUpHandler() {
@@ -3149,6 +3154,13 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             getEscalator().getBody().refreshRows(0,
                     getEscalator().getBody().getRowCount());
         }
+
+        private void cleanup() {
+            if (headerClickHandler != null) {
+                headerClickHandler.removeHandler();
+                headerClickHandler = null;
+            }
+        }
     }
 
     /**
@@ -7927,6 +7939,10 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             return;
         }
 
+        if (this.selectionColumn != null) {
+            selectionColumn.cleanup();
+        }
+
         if (this.selectColumnRenderer != null) {
             if (this.selectColumnRenderer instanceof ComplexRenderer) {
                 // End of Life for the old selection column renderer.
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSort.java b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSort.java
new file mode 100644
index 0000000000..f62276de2c
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSort.java
@@ -0,0 +1,35 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.minitutorials.v7_4.GridExampleHelper;
+import com.vaadin.ui.Button;
+import com.vaadin.v7.ui.Grid;
+import com.vaadin.v7.ui.Grid.MultiSelectionModel;
+import com.vaadin.v7.ui.Grid.SelectionMode;
+
+public class CompatibilityGridToggleMultiSelectSort extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        // container with at least 100 rows
+        final Grid grid = new Grid(GridExampleHelper.createContainer());
+        grid.setSelectionMode(SelectionMode.MULTI);
+        addComponent(grid);
+
+        Button button = new Button("Toggle multi-select", e -> {
+            if (grid.getSelectionModel() instanceof MultiSelectionModel) {
+                grid.setSelectionMode(SelectionMode.SINGLE);
+            } else {
+                grid.setSelectionMode(SelectionMode.MULTI);
+            }
+        });
+        addComponent(button);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Toggling multi-select off should not break sorting "
+                + "first column to both directions.";
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSortTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSortTest.java
new file mode 100644
index 0000000000..e95c3d0792
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridToggleMultiSelectSortTest.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class CompatibilityGridToggleMultiSelectSortTest
+        extends SingleBrowserTest {
+
+    @Test
+    public void sortFirstColumnAfterToggle() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        ButtonElement button = $(ButtonElement.class).first();
+
+        button.click();
+
+        assertEquals("Unexpected initial sorting.", "0",
+                grid.getCell(0, 0).getText());
+
+        GridCellElement headerCell = grid.getHeaderCell(0, 0);
+
+        // sort ascending
+        headerCell.click();
+        assertEquals("Unexpected first sorting.", "0",
+                grid.getCell(0, 0).getText());
+
+        // sort descending
+        headerCell.click();
+        assertEquals("Unexpected second sorting.", "99.9",
+                grid.getCell(0, 0).getText());
+    }
+
+}