Fixes a Grid issue that breaks sorting. (#10999)

* Fixes a Grid issue that breaks sorting.

When multi-select has been enabled and disabled a number of times, sorting on the first column in a Grid is broken.
Unregistering a click handler on the header solves the problem.

* Added a test checking that sorting of a grid column still works after having enabled and then disabled multi select mode.
The grid is configured with an extra header row containing a text box since it triggers an exception if the header click handler for the multi select column is not removed.

* Remove an unused import that was added by mistake

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 06565e13f2b928615f56591d0dbdcacfa7405da4..7ef61eff5b13aef34000589fcefc77e8a2edd6ba 100755 (executable)
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -2937,6 +2937,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);
@@ -2990,7 +2991,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
                         });
                 selectAllCheckBox.setValue(selected);
 
-                addHeaderClickHandler(new HeaderClickHandler() {
+                headerClickHandler = addHeaderClickHandler(new HeaderClickHandler() {
                     @Override
                     public void onClick(GridClickEvent event) {
                         if (!userSelectionAllowed) {
@@ -3146,6 +3147,13 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
             getEscalator().getBody().refreshRows(0,
                     getEscalator().getBody().getRowCount());
         }
+
+        public void cleanup() {
+            if (headerClickHandler != null) {
+                headerClickHandler.removeHandler();
+                headerClickHandler = null;
+            }
+        }
     }
 
     /**
@@ -7927,6 +7935,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/widgetset/client/grid/GridBasicClientFeaturesWidget.java b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
index cc8fa9e39fe9592c08247c28523a245323303de7..ada9e66ef40af068b5ffce439113e8792bb80c2d 100644 (file)
--- a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
+++ b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
@@ -1138,6 +1138,13 @@ public class GridBasicClientFeaturesWidget
 
             }
         }, menuPath);
+
+        addMenuCommand("Set widget", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                row.getCell(grid.getColumn(0)).setWidget(new TextBox());
+            }
+        }, menuPath);
     }
 
     private void createFooterMenu() {

diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/client/GridHeaderTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/client/GridHeaderTest.java
index 8806a35043c9074ff59fb85fbdc43f595b2c3427..31c107d52452b3d7c0cd1a099cb710a922cb8d08 100644 (file)
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/client/GridHeaderTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/client/GridHeaderTest.java
@@ -256,6 +256,23 @@ public class GridHeaderTest extends GridStaticSectionTest {
         assertEquals("clicked", button.getText().toLowerCase());
     }
 
+    @Test
+    public void testSortAfterMultiSelect() throws Exception {
+        openTestURL();
+
+        selectMenuPath("Component", "Columns", "Column 0", "Sortable");
+        selectMenuPath("Component", "Header", "Append row");
+        selectMenuPath("Component", "Header", "Row 2", "Set widget");
+        selectMenuPath("Component", "State", "Selection mode", "multi");
+        selectMenuPath("Component", "State", "Selection mode", "single");
+
+        GridCellElement headerCell = getGridElement().getHeaderCell(0, 0);
+
+        headerCell.click();
+
+        assertTrue(hasClassName(headerCell, "sort-asc"));
+    }
+
     private void assertHeaderCount(int count) {
         assertEquals("header count", count, getGridElement().getHeaderCount());
     }