Refactored AxisSyncer and moved some code to more generic place && fi…

…xed problems related to ManualRowMove and ManualColumnMove plugins && e2e tests

handsontable/src/helpers/moves.js

@@ -0,0 +1,92 @@
+/**
+ * Gets first position where to move element (respecting the fact that some element will be sooner or later
+ * taken out of the dataset in order to move them).
+ *
+ * @param {Array<number>} movedIndexes Sequence of moved indexes for certain axis.
+ * @param {number} finalIndex Final place where to move rows.
+ * @param {number} numberOfIndexes Number of indexes in a dataset.
+ * @returns {number} Index informing where to move the first element.
+ */
+function getMoveLine(movedIndexes, finalIndex, numberOfIndexes) {
+  const notMovedElements = Array.from(Array(numberOfIndexes).keys())
+    .filter(index => movedIndexes.includes(index) === false);
+
+  if (finalIndex === 0) {
+    return notMovedElements[finalIndex] ?? 0; // Moving before the first dataset's element.
+  }
+
+  return notMovedElements[finalIndex - 1] + 1; // Moving before another element.
+}
+
+/**
+ * Gets initially calculated move positions.
+ *
+ * @param {Array<number>} movedIndexes Sequence of moved indexes for certain axis.
+ * @param {number} moveLine Final place where to move rows.
+ * @returns {Array<{from: number, to: number}>} Initially calculated move positions.
+ */
+function getInitiallyCalculatedMoves(movedIndexes, moveLine) {
+  const moves = [];
+
+  movedIndexes.forEach((movedIndex) => {
+    const move = {
+      from: movedIndex,
+      to: moveLine,
+    };
+
+    moves.forEach((previouslyMovedIndex) => {
+      const isMovingFromEndToStart = previouslyMovedIndex.from > previouslyMovedIndex.to;
+      const isMovingElementBefore = previouslyMovedIndex.to <= move.from;
+      const isMovingAfterElement = previouslyMovedIndex.from > move.from;
+
+      if (isMovingAfterElement && isMovingElementBefore && isMovingFromEndToStart) {
+        move.from += 1;
+      }
+    });
+
+    // Moved element from right to left (or bottom to top).
+    if (move.from >= moveLine) {
+      moveLine += 1;
+    }
+
+    moves.push(move);
+  });
+
+  return moves;
+}
+
+/**
+ * Gets finally calculated move positions (after adjusting).
+ *
+ * @param {Array<{from: number, to: number}>} moves Initially calculated move positions.
+ * @returns {Array<{from: number, to: number}>} Finally calculated move positions (after adjusting).
+ */
+function adjustedCalculatedMoves(moves) {
+  moves.forEach((move, index) => {
+    const nextMoved = moves.slice(index + 1);
+
+    nextMoved.forEach((nextMovedIndex) => {
+      const isMovingFromStartToEnd = nextMovedIndex.from < nextMovedIndex.to;
+
+      if (nextMovedIndex.from > move.from && isMovingFromStartToEnd) {
+        nextMovedIndex.from -= 1;
+      }
+    });
+  });
+
+  return moves;
+}
+
+/**
+ * Get list of move positions.
+ *
+ * @param {Array<number>} movedIndexes Sequence of moved indexes for certain axis.
+ * @param {number} finalIndex Final place where to move rows.
+ * @param {number} numberOfIndexes Number of indexes in a dataset.
+ * @returns {Array<{from: number, to: number}>}
+ */
+export function getMoves(movedIndexes, finalIndex, numberOfIndexes) {
+  const moves = getInitiallyCalculatedMoves(movedIndexes, getMoveLine(movedIndexes, finalIndex, numberOfIndexes));
+
+  return adjustedCalculatedMoves(moves);
+}

handsontable/src/plugins/formulas/indexSyncer/axisSyncer.js

@@ -1,4 +1,5 @@
 import { toUpperCaseFirst } from '../../../helpers/string';
+import { getMoves } from '../../../helpers/moves';
 
 /**
  * @private
@@ -145,89 +146,6 @@ class AxisSyncer {
     this.#finalIndex = this.getHfIndexFromVisualIndex(visualFinalIndex);
   }
 
-  /**
-   * Gets first position where to move element (respecting the fact that some element will be sooner or later
-   * taken out of the dataset in order to move them).
-   *
-   * @param {Array<number>} movedHfIndexes Sequence of moved HF indexes for certain axis.
-   * @param {number} finalHfIndex Final HF place where to move rows.
-   * @returns {number} HF's index informing where to move the first element.
-   * @private
-   */
-  getMoveLine(movedHfIndexes, finalHfIndex) {
-    const numberOfElements = this.#indexMapper.getNumberOfIndexes();
-    const notMovedElements = Array.from(Array(numberOfElements).keys())
-      .filter(index => movedHfIndexes.includes(index) === false);
-
-    if (finalHfIndex === 0) {
-      return notMovedElements[finalHfIndex] ?? 0; // Moving before the first dataset's element.
-    }
-
-    return notMovedElements[finalHfIndex - 1] + 1; // Moving before another element.
-  }
-
-  /**
-   * Gets initially calculated HF's move positions.
-   *
-   * @private
-   * @param {Array<number>} movedHfIndexes Sequence of moved HF indexes for certain axis.
-   * @param {number} finalHfIndex Final HF place where to move rows.
-   * @returns {Array<{from: number, to: number}>} Initially calculated HF's move positions.
-   */
-  getInitiallyCalculatedMoves(movedHfIndexes, finalHfIndex) {
-    let moveLine = this.getMoveLine(movedHfIndexes, finalHfIndex);
-    const moves = [];
-
-    movedHfIndexes.forEach((movedHfIndex) => {
-      const move = {
-        from: movedHfIndex,
-        to: moveLine,
-      };
-
-      moves.forEach((previouslyMovedIndex) => {
-        const isMovingFromEndToStart = previouslyMovedIndex.from > previouslyMovedIndex.to;
-        const isMovingElementBefore = previouslyMovedIndex.to <= move.from;
-        const isMovingAfterElement = previouslyMovedIndex.from > move.from;
-
-        if (isMovingAfterElement && isMovingElementBefore && isMovingFromEndToStart) {
-          move.from += 1;
-        }
-      });
-
-      // Moved element from right to left (or bottom to top).
-      if (move.from >= moveLine) {
-        moveLine += 1;
-      }
-
-      moves.push(move);
-    });
-
-    return moves;
-  }
-
-  /**
-   * Gets finally calculated HF's move positions (after adjusting).
-   *
-   * @private
-   * @param {Array<{from: number, to: number}>} moves Initially calculated HF's move positions.
-   * @returns {Array<{from: number, to: number}>} Finally calculated HF's move positions (after adjusting).
-   */
-  adjustedCalculatedMoves(moves) {
-    moves.forEach((move, index) => {
-      const nextMoved = moves.slice(index + 1);
-
-      nextMoved.forEach((nextMovedIndex) => {
-        const isMovingFromStartToEnd = nextMovedIndex.from < nextMovedIndex.to;
-
-        if (nextMovedIndex.from > move.from && isMovingFromStartToEnd) {
-          nextMovedIndex.from -= 1;
-        }
-      });
-    });
-
-    return moves;
-  }
-
   /**
    * Calculating where to move HF elements and performing already calculated moves.
    *
@@ -243,9 +161,7 @@ class AxisSyncer {
       return;
     }
 
-    const calculatedMoves = this.adjustedCalculatedMoves(
-      this.getInitiallyCalculatedMoves(this.#movedIndexes, this.#finalIndex)
-    );
+    const calculatedMoves = getMoves(this.#movedIndexes, this.#finalIndex, this.#indexMapper.getNumberOfIndexes());
 
     if (this.#indexSyncer.getSheetId() === null) {
       this.#indexSyncer.getPostponeAction(() => this.syncMoves(calculatedMoves));

handsontable/src/plugins/manualColumnMove/__tests__/manualColumnMove.spec.js

@@ -1411,6 +1411,26 @@ describe('manualColumnMove', () => {
 
         expect(hot.getDataAtRow(0)).toEqual(['A1', 'B1', 'C1', 'D1', 'E1', 'F1', 'G1', 'H1', 'I1', 'J1']);
       });
+
+      it('when moving using few actions', () => {
+        const hot = handsontable({
+          data: Handsontable.helper.createSpreadsheetData(10, 10),
+          colHeaders: true,
+          manualColumnMove: true,
+        });
+
+        hot.getPlugin('manualColumnMove').moveColumn(0, 9);
+        hot.getPlugin('manualColumnMove').moveColumn(0, 9);
+        hot.render();
+
+        hot.undo();
+
+        expect(hot.getDataAtRow(0)).toEqual(['B1', 'C1', 'D1', 'E1', 'F1', 'G1', 'H1', 'I1', 'J1', 'A1']);
+
+        hot.undo();
+
+        expect(hot.getDataAtRow(0)).toEqual(['A1', 'B1', 'C1', 'D1', 'E1', 'F1', 'G1', 'H1', 'I1', 'J1']);
+      });
     });
 
     describe('should revert changes', () => {
@@ -1477,6 +1497,29 @@ describe('manualColumnMove', () => {
 
         expect(hot.getDataAtRow(0)).toEqual(['C1', 'D1', 'A1', 'B1', 'I1', 'E1', 'H1', 'F1', 'G1', 'J1']);
       });
+
+      it('when moving using few actions', () => {
+        const hot = handsontable({
+          data: Handsontable.helper.createSpreadsheetData(10, 10),
+          colHeaders: true,
+          manualColumnMove: true,
+        });
+
+        hot.getPlugin('manualColumnMove').moveColumn(0, 9);
+        hot.getPlugin('manualColumnMove').moveColumn(0, 9);
+        hot.render();
+
+        hot.undo();
+        hot.undo();
+
+        hot.redo();
+
+        expect(hot.getDataAtRow(0)).toEqual(['B1', 'C1', 'D1', 'E1', 'F1', 'G1', 'H1', 'I1', 'J1', 'A1']);
+
+        hot.redo();
+
+        expect(hot.getDataAtRow(0)).toEqual(['C1', 'D1', 'E1', 'F1', 'G1', 'H1', 'I1', 'J1', 'A1', 'B1']);
+      });
     });
   });
 });

handsontable/src/plugins/manualRowMove/__tests__/manualRowMove.spec.js

@@ -897,6 +897,26 @@ describe('manualRowMove', () => {
 
         expect(hot.getDataAtCol(0)).toEqual(['A1', 'A2', 'A3', 'A4', 'A5', 'A6', 'A7', 'A8', 'A9', 'A10']);
       });
+
+      it('when moving using few actions', () => {
+        const hot = handsontable({
+          data: Handsontable.helper.createSpreadsheetData(10, 10),
+          rowHeaders: true,
+          manualRowMove: true,
+        });
+
+        hot.getPlugin('manualRowMove').moveRow(0, 9);
+        hot.getPlugin('manualRowMove').moveRow(0, 9);
+        hot.render();
+
+        hot.undo();
+
+        expect(hot.getDataAtCol(0)).toEqual(['A2', 'A3', 'A4', 'A5', 'A6', 'A7', 'A8', 'A9', 'A10', 'A1']);
+
+        hot.undo();
+
+        expect(hot.getDataAtCol(0)).toEqual(['A1', 'A2', 'A3', 'A4', 'A5', 'A6', 'A7', 'A8', 'A9', 'A10']);
+      });
     });
 
     describe('should revert changes', () => {
@@ -963,6 +983,29 @@ describe('manualRowMove', () => {
 
         expect(hot.getDataAtCol(0)).toEqual(['A3', 'A4', 'A1', 'A2', 'A9', 'A5', 'A8', 'A6', 'A7', 'A10']);
       });
+
+      it('when moving using few actions', () => {
+        const hot = handsontable({
+          data: Handsontable.helper.createSpreadsheetData(10, 10),
+          rowHeaders: true,
+          manualRowMove: true,
+        });
+
+        hot.getPlugin('manualRowMove').moveRow(0, 9);
+        hot.getPlugin('manualRowMove').moveRow(0, 9);
+        hot.render();
+
+        hot.undo();
+        hot.undo();
+
+        hot.redo();
+
+        expect(hot.getDataAtCol(0)).toEqual(['A2', 'A3', 'A4', 'A5', 'A6', 'A7', 'A8', 'A9', 'A10', 'A1']);
+
+        hot.redo();
+
+        expect(hot.getDataAtCol(0)).toEqual(['A3', 'A4', 'A5', 'A6', 'A7', 'A8', 'A9', 'A10', 'A1', 'A2', ]);
+      });
     });
   });
 

handsontable/src/plugins/undoRedo/undoRedo.js

@@ -3,6 +3,7 @@ import { arrayMap, arrayEach } from '../../helpers/array';
 import { rangeEach } from '../../helpers/number';
 import { inherit, deepClone } from '../../helpers/object';
 import { align } from '../contextMenu/utils';
+import { getMoves } from '../../helpers/moves';
 
 const SHORTCUTS_GROUP = 'undoRedo';
 
@@ -902,23 +903,20 @@ inherit(UndoRedo.RowMoveAction, UndoRedo.Action);
 
 UndoRedo.RowMoveAction.prototype.undo = function(instance, undoneCallback) {
   const manualRowMove = instance.getPlugin('manualRowMove');
-  const copyOfRows = [].concat(this.rows);
-  const rowsMovedUp = copyOfRows.filter(a => a > this.finalRowIndex);
-  const rowsMovedDown = copyOfRows.filter(a => a <= this.finalRowIndex);
-  const allMovedRows = rowsMovedUp.sort((a, b) => b - a).concat(rowsMovedDown.sort((a, b) => a - b));
 
   instance.addHookOnce('afterViewRender', undoneCallback);
 
-  // Moving rows from those with higher indexes to those with lower indexes when action was performed from bottom to top
-  // Moving rows from those with lower indexes to those with higher indexes when action was performed from top to bottom
-  for (let i = 0; i < allMovedRows.length; i += 1) {
-    const newPhysicalRow = instance.toVisualRow(allMovedRows[i]);
+  const rowMoves = getMoves(this.rows, this.finalRowIndex, instance.rowIndexMapper.getNumberOfIndexes());
 
-    manualRowMove.moveRow(newPhysicalRow, allMovedRows[i]);
-  }
+  rowMoves.reverse().forEach(({ from, to }) => {
+    if (from < to) {
+      to -= 1;
+    }
 
-  instance.render();
+    manualRowMove.moveRow(to, from);
+  });
 
+  instance.render();
   instance.deselectCell();
   instance.selectRows(this.rows[0], this.rows[0] + this.rows.length - 1);
 };
@@ -949,23 +947,20 @@ inherit(UndoRedo.ColumnMoveAction, UndoRedo.Action);
 
 UndoRedo.ColumnMoveAction.prototype.undo = function(instance, undoneCallback) {
   const manualColumnMove = instance.getPlugin('manualColumnMove');
-  const copyOfColumns = [].concat(this.columns);
-  const columnsMovedLeft = copyOfColumns.filter(a => a > this.finalColumnIndex);
-  const rowsMovedRight = copyOfColumns.filter(a => a <= this.finalColumnIndex);
-  const allMovedColumns = columnsMovedLeft.sort((a, b) => b - a).concat(rowsMovedRight.sort((a, b) => a - b));
 
   instance.addHookOnce('afterViewRender', undoneCallback);
 
-  // Moving columns from those with higher indexes to those with lower indexes when action was performed from right to left
-  // Moving columns from those with lower indexes to those with higher indexes when action was performed from left to right
-  for (let i = 0; i < allMovedColumns.length; i += 1) {
-    const newPhysicalColumn = instance.toVisualColumn(allMovedColumns[i]);
+  const columnMoves = getMoves(this.columns, this.finalColumnIndex, instance.columnIndexMapper.getNumberOfIndexes());
 
-    manualColumnMove.moveColumn(newPhysicalColumn, allMovedColumns[i]);
-  }
+  columnMoves.reverse().forEach(({ from, to }) => {
+    if (from < to) {
+      to -= 1;
+    }
 
-  instance.render();
+    manualColumnMove.moveColumn(to, from);
+  });
 
+  instance.render();
   instance.deselectCell();
   instance.selectColumns(this.columns[0], this.columns[0] + this.columns.length - 1);
 };