From 3234e136ece55271454259762de348308ef17798 Mon Sep 17 00:00:00 2001
From: Connor Yoh <con.yoh13@gmail.com>
Date: Fri, 28 Feb 2025 16:40:54 +0000
Subject: [PATCH] Simplified movepageup/down to movepage in multitool. undo now
 works for any index change

---
 .../static/js/multitool/PdfActionsManager.js  |  6 +--
 .../static/js/multitool/PdfContainer.js       | 30 ++++--------
 .../static/js/multitool/commands/move-page.js | 49 ++-----------------
 3 files changed, 16 insertions(+), 69 deletions(-)

diff --git a/src/main/resources/static/js/multitool/PdfActionsManager.js b/src/main/resources/static/js/multitool/PdfActionsManager.js
index 834247940..96adafa1f 100644
--- a/src/main/resources/static/js/multitool/PdfActionsManager.js
+++ b/src/main/resources/static/js/multitool/PdfActionsManager.js
@@ -35,8 +35,7 @@ class PdfActionsManager {
 
     const sibling = imgContainer.previousSibling;
     if (sibling) {
-      let movePageCommand = this.movePageTo(imgContainer, sibling, true, true);
-      this._pushUndoClearRedo(movePageCommand);
+      this.movePageTo(imgContainer, sibling, true);
     }
   }
 
@@ -44,12 +43,11 @@ class PdfActionsManager {
     var imgContainer = this.getPageContainer(e.target);
     const sibling = imgContainer.nextSibling;
     if (sibling) {
-      let movePageCommand = this.movePageTo(
+      this.movePageTo(
         imgContainer,
         sibling.nextSibling,
         true
       );
-      this._pushUndoClearRedo(movePageCommand);
     }
   }
 
diff --git a/src/main/resources/static/js/multitool/PdfContainer.js b/src/main/resources/static/js/multitool/PdfContainer.js
index 86bdb55f4..3ea652760 100644
--- a/src/main/resources/static/js/multitool/PdfContainer.js
+++ b/src/main/resources/static/js/multitool/PdfContainer.js
@@ -1,4 +1,4 @@
-import { MovePageUpCommand, MovePageDownCommand } from './commands/move-page.js';
+import { MovePageCommand } from './commands/move-page.js';
 import { RemoveSelectedCommand } from './commands/remove.js';
 import { RotateAllCommand, RotateElementCommand } from './commands/rotate.js';
 import { SplitAllCommand } from './commands/split.js';
@@ -109,27 +109,17 @@ class PdfContainer {
     downloadBtn.disabled = true;
   }
 
-  movePageTo(startElement, endElement, scrollTo = false, moveUp = false) {
-    let movePageCommand;
-    if (moveUp) {
-      movePageCommand = new MovePageUpCommand(
-        startElement,
-        endElement,
-        this.pagesContainer,
-        this.pagesContainerWrapper,
-        scrollTo
-      );
-    } else {
-      movePageCommand = new MovePageDownCommand(
-        startElement,
-        endElement,
-        this.pagesContainer,
-        this.pagesContainerWrapper,
-        scrollTo
-      );
-    }
+  movePageTo(startElement, endElement, scrollTo = false) {
+    let movePageCommand = new MovePageCommand(
+      startElement,
+      endElement,
+      this.pagesContainer,
+      this.pagesContainerWrapper,
+      scrollTo
+    );
 
     movePageCommand.execute();
+    this.undoManager.pushUndoClearRedo(movePageCommand);
     return movePageCommand;
   }
 
diff --git a/src/main/resources/static/js/multitool/commands/move-page.js b/src/main/resources/static/js/multitool/commands/move-page.js
index 738af9502..b209e6a7d 100644
--- a/src/main/resources/static/js/multitool/commands/move-page.js
+++ b/src/main/resources/static/js/multitool/commands/move-page.js
@@ -1,6 +1,6 @@
 import {Command} from './command.js';
 
-export class AbstractMovePageCommand extends Command {
+export class MovePageCommand extends Command {
   constructor(startElement, endElement, pagesContainer, pagesContainerWrapper, scrollTo = false) {
     super();
 
@@ -16,7 +16,6 @@ export class AbstractMovePageCommand extends Command {
     this.scrollTo = scrollTo;
     this.pagesContainerWrapper = pagesContainerWrapper;
   }
-
   execute() {
     // Check & remove page number elements here too if they exist because Firefox doesn't fire the relevant event on page move.
     const pageNumberElement = this.startElement.querySelector('.page-number');
@@ -42,51 +41,11 @@ export class AbstractMovePageCommand extends Command {
   }
 
   undo() {
-    // Requires overriding in child classes
-  }
-
-  redo() {
-    this.execute();
-  }
-}
-
-export class MovePageUpCommand extends AbstractMovePageCommand {
-  constructor(startElement, endElement, pagesContainer, pagesContainerWrapper, scrollTo = false) {
-    super(startElement, endElement, pagesContainer, pagesContainerWrapper, scrollTo);
-  }
-
-  undo() {
-    if (this.endElement) {
-      this.pagesContainer.removeChild(this.endElement);
-      this.startElement.insertAdjacentElement('beforebegin', this.endElement);
-    }
-
-    if (this.scrollTo) {
-      const {width} = this.startElement.getBoundingClientRect();
-      const vector = this.endIndex === -1 || this.startIndex <= this.endIndex ? 0 - width : width;
-
-      this.pagesContainerWrapper.scroll({
-        left: this.pagesContainerWrapper.scrollLeft - vector,
-      });
-    }
-  }
-
-  redo() {
-    this.execute();
-  }
-}
-
-export class MovePageDownCommand extends AbstractMovePageCommand {
-  constructor(startElement, endElement, pagesContainer, pagesContainerWrapper, scrollTo = false) {
-    super(startElement, endElement, pagesContainer, pagesContainerWrapper, scrollTo);
-  }
-
-  undo() {
-    let previousElement = this.startElement.previousSibling;
-
     if (this.startElement) {
       this.pagesContainer.removeChild(this.startElement);
-      previousElement.insertAdjacentElement('beforebegin', this.startElement);
+      let previousNeighbour = Array.from(this.pagesContainer.childNodes)[this.startIndex];
+      previousNeighbour?.insertAdjacentElement('beforebegin', this.startElement)
+        ?? this.pagesContainer.append(this.startElement);
     }
 
     if (this.scrollTo) {