afterColumnMove hook is enable only when beforeColumnMove not returns…

… false #5958 (#6078)
handsontable · Jul 8, 2019 · 3ebea4c · 3ebea4c
1 parent cab32fe
commit 3ebea4c
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/src/plugins/manualColumnMove/manualColumnMove.js b/src/plugins/manualColumnMove/manualColumnMove.js
@@ -210,9 +210,9 @@ class ManualColumnMove extends BasePlugin {
 
       // after moving we have to clear columnsMapper from null entries
       this.columnsMapper.clearNull();
-    }
 
-    this.hot.runHooks('afterColumnMove', visualColumns, target);
+      this.hot.runHooks('afterColumnMove', visualColumns, target);
+    }
   }
 
   /**

diff --git a/src/plugins/manualColumnMove/test/manualColumnMove.e2e.js b/src/plugins/manualColumnMove/test/manualColumnMove.e2e.js
@@ -285,6 +285,33 @@ describe('manualColumnMove', () => {
       expect(afterMoveColumnCallback).toHaveBeenCalledWith([8, 9, 7], 0, void 0, void 0, void 0, void 0);
     });
 
+    it('should not trigger an afterColumnMove event after column move when beforeColumnMove returns false', () => {
+      const afterMoveColumnCallback = jasmine.createSpy('afterMoveColumnCallback');
+
+      spec().$container.height(150);
+
+      const hot = handsontable({
+        data: Handsontable.helper.createSpreadsheetData(10, 10),
+        colHeaders: true,
+        manualColumnMove: true,
+        beforeColumnMove: () => false,
+        afterColumnMove: afterMoveColumnCallback
+      });
+
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(0)').text()).toEqual('A1');
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(1)').text()).toEqual('B1');
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(2)').text()).toEqual('C1');
+
+      hot.getPlugin('manualColumnMove').moveColumns([8, 9, 7], 0);
+      hot.render();
+
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(0)').text()).toEqual('A1');
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(1)').text()).toEqual('B1');
+      expect(spec().$container.find('tbody tr:eq(0) td:eq(2)').text()).toEqual('C1');
+
+      expect(afterMoveColumnCallback).not.toHaveBeenCalled();
+    });
+
     it('should move the second column to the first column', () => {
       handsontable({
         data: Handsontable.helper.createSpreadsheetData(10, 10),