Merge pull request #5525 from dlgski/colMoveVisible

fix: prevent hidden columns triggering unnecessary re-order

Author: AgDude

Diff for: src/features/move-columns/js/column-movable.js

@@ -175,13 +175,14 @@
         }
 
         //check columns in between move-range to make sure they are visible columns
-        var i0 = Math.min(originalPosition, newPosition);
-        for (i0; i0 < Math.max(originalPosition, newPosition);i0++) {
+        var pos = (originalPosition < newPosition) ? originalPosition + 1 : originalPosition - 1;
+        var i0 = Math.min(pos, newPosition);
+        for (i0; i0 <= Math.max(pos, newPosition); i0++) {
           if (columns[i0].visible) {
             break;
           }
         }
-        if (i0 === Math.max(originalPosition, newPosition)) {
+        if (i0 > Math.max(pos, newPosition)) {
           //no visible column found, column did not visibly move
           return;
         }

Diff for: src/features/move-columns/test/column-movable.spec.js

@@ -183,5 +183,15 @@ describe('ui.grid.moveColumns', function () {
     expect(scope.grid.columns[3].name).toBe('company');
     expect(scope.grid.columns[4].name).toBe('phone');
   });
-
+  
+  it('expect column move not to happen if moving across hidden columns', function() {
+    scope.gridOptions.columnDefs[1].visible = false;
+    scope.gridApi.colMovable.moveColumn(0, 3);
+    expect(scope.grid.columns[0].name).toBe('name');
+    expect(scope.grid.columns[1].name).toBe('gender');
+    expect(scope.grid.columns[2].name).toBe('age');
+    expect(scope.grid.columns[3].name).toBe('company');
+    expect(scope.grid.columns[4].name).toBe('phone');
+  });
+  
 });