Merge pull request #214 from vaadin/fix/event-order-on-sort

Fixed order of events on column sort (fixes #206)

CHANGES.md

@@ -12,6 +12,7 @@
   - Hiding the header/footer declarative requires the element to have at least one cell (#135)
   - JSRow reference data with single column ds fixed
   - Column header text should default to name if none is given (#136)
+  - Fire sort event before clearing the selection (#206)
 
   #### API changes:
   - `grid.data.source` is now `grid.items` for Arrays and `grid.datasource` for Functions. (#160)

java/src/main/java/com/vaadin/elements/grid/GridElement.java

@@ -512,13 +512,12 @@ private JSArray<JSSortOrder> mapToJSSortOrders(List<SortOrder> sortOrders) {
 
     @Override
     public void sort(SortEvent<Object> event) {
-        getSelectionModel().reset();
-
         if (event.isUserOriginated()) {
             JsUtils.prop(container, "sortOrder",
                     mapToJSSortOrders(event.getOrder()));
         }
 
+        getSelectionModel().reset();
         clearDataSourceCache();
     }
 

test/grid-sorting-rows.html

@@ -17,6 +17,18 @@
   <script>
     describe.feature('sorting rows', function() {
 
+      var sortListener;
+
+      before(function() {
+        grid.addEventListener('sort-order-changed', function() {
+          sortListener();
+        });
+      });
+
+      beforeEach(function() {
+        sortListener = sinon.spy();
+      });
+
       describe('using api', function() {
 
         function assignInvalidSortOrder() {
@@ -90,6 +102,19 @@
           expect(grid.selection.selected()).to.be.empty;
         });
 
+        it('should fire sort event before clearing the selection on api sort', function(done) {
+          sortListener = function() {
+              expect(grid.selection.selected()).to.eql([0]);
+              done();
+          };
+
+          grid.selection.select(0);
+          grid.sortOrder = [{
+            column: 0,
+            direction: "desc"
+          }];
+        });
+
         it('should ignore selection column from index', function() {
           var firstNonFrozenHeaderCell = qLocal.bind(this, ".vaadin-grid-header .vaadin-grid-cell:not(.frozen)");
 
@@ -161,6 +186,17 @@
           }]);
         });
 
+        it('should fire sort event before clearing the selection on user sort', function(done) {
+          sortListener = function() {
+            expect(grid.selection.selected()).to.eql([0]);
+            done();
+          };
+
+          grid.selection.select(0);
+          var cells = qaLocal(".vaadin-grid-header .vaadin-grid-cell");
+          triggerMouseEvent(cells[1], "click");
+        });
+
         it('should sort by two columns', function() {
           grid.sortOrder = [];
           var cells = qaLocal(".vaadin-grid-header .vaadin-grid-cell");
@@ -178,5 +214,14 @@
         });
       });
 
+      it('should clear selection after sort', function() {
+        grid.selection.select(0);
+        grid.sortOrder = [{
+          column: 0,
+          direction: "desc"
+        }];
+        expect(grid.selection.selected()).to.be.empty;
+      });
+
     });
   </script>