Merge pull request #17564 from calixteman/editor_update_ui_undo

[Editor] Update the parameters in the UI of the last selected editor when undoing/redoing

src/display/editor/freetext.js

@@ -224,12 +224,9 @@ class FreeTextEditor extends AnnotationEditor {
     };
     const savedFontsize = this.#fontSize;
     this.addCommands({
-      cmd: () => {
+      cmd: setFontsize.bind(this, fontSize),
-        setFontsize(fontSize);
+      undo: setFontsize.bind(this, savedFontsize),
-      },
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-      undo: () => {
-        setFontsize(savedFontsize);
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.FREETEXT_SIZE,
       overwriteIfSameType: true,
@@ -242,14 +239,14 @@ class FreeTextEditor extends AnnotationEditor {
    * @param {string} color
    */
   #updateColor(color) {
+    const setColor = col => {
+      this.#color = this.editorDiv.style.color = col;
+    };
     const savedColor = this.#color;
     this.addCommands({
-      cmd: () => {
+      cmd: setColor.bind(this, color),
-        this.#color = this.editorDiv.style.color = color;
+      undo: setColor.bind(this, savedColor),
-      },
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-      undo: () => {
-        this.#color = this.editorDiv.style.color = savedColor;
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.FREETEXT_COLOR,
       overwriteIfSameType: true,

src/display/editor/highlight.js

@@ -221,18 +221,16 @@ class HighlightEditor extends AnnotationEditor {
    * @param {string} color
    */
   #updateColor(color) {
+    const setColor = col => {
+      this.color = col;
+      this.parent?.drawLayer.changeColor(this.#id, col);
+      this.#colorPicker?.updateColor(col);
+    };
     const savedColor = this.color;
     this.addCommands({
-      cmd: () => {
+      cmd: setColor.bind(this, color),
-        this.color = color;
+      undo: setColor.bind(this, savedColor),
-        this.parent?.drawLayer.changeColor(this.#id, color);
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-        this.#colorPicker?.updateColor(color);
-      },
-      undo: () => {
-        this.color = savedColor;
-        this.parent?.drawLayer.changeColor(this.#id, savedColor);
-        this.#colorPicker?.updateColor(savedColor);
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.HIGHLIGHT_COLOR,
       overwriteIfSameType: true,

src/display/editor/ink.js

@@ -158,16 +158,15 @@ class InkEditor extends AnnotationEditor {
    * @param {number} thickness
    */
   #updateThickness(thickness) {
+    const setThickness = th => {
+      this.thickness = th;
+      this.#fitToContent();
+    };
     const savedThickness = this.thickness;
     this.addCommands({
-      cmd: () => {
+      cmd: setThickness.bind(this, thickness),
-        this.thickness = thickness;
+      undo: setThickness.bind(this, savedThickness),
-        this.#fitToContent();
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-      },
-      undo: () => {
-        this.thickness = savedThickness;
-        this.#fitToContent();
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.INK_THICKNESS,
       overwriteIfSameType: true,
@@ -180,16 +179,15 @@ class InkEditor extends AnnotationEditor {
    * @param {string} color
    */
   #updateColor(color) {
+    const setColor = col => {
+      this.color = col;
+      this.#redraw();
+    };
     const savedColor = this.color;
     this.addCommands({
-      cmd: () => {
+      cmd: setColor.bind(this, color),
-        this.color = color;
+      undo: setColor.bind(this, savedColor),
-        this.#redraw();
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-      },
-      undo: () => {
-        this.color = savedColor;
-        this.#redraw();
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.INK_COLOR,
       overwriteIfSameType: true,
@@ -202,17 +200,16 @@ class InkEditor extends AnnotationEditor {
    * @param {number} opacity
    */
   #updateOpacity(opacity) {
+    const setOpacity = op => {
+      this.opacity = op;
+      this.#redraw();
+    };
     opacity /= 100;
     const savedOpacity = this.opacity;
     this.addCommands({
-      cmd: () => {
+      cmd: setOpacity.bind(this, opacity),
-        this.opacity = opacity;
+      undo: setOpacity.bind(this, savedOpacity),
-        this.#redraw();
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
-      },
-      undo: () => {
-        this.opacity = savedOpacity;
-        this.#redraw();
-      },
       mustExec: true,
       type: AnnotationEditorParamsType.INK_OPACITY,
       overwriteIfSameType: true,

src/display/editor/tools.js

@@ -244,6 +244,7 @@ class CommandManager {
    * @typedef {Object} addOptions
    * @property {function} cmd
    * @property {function} undo
+   * @property {function} [post]
    * @property {boolean} mustExec
    * @property {number} type
    * @property {boolean} overwriteIfSameType
@@ -257,6 +258,7 @@ class CommandManager {
   add({
     cmd,
     undo,
+    post,
     mustExec,
     type = NaN,
     overwriteIfSameType = false,
@@ -270,7 +272,7 @@ class CommandManager {
       return;
     }
 
-    const save = { cmd, undo, type };
+    const save = { cmd, undo, post, type };
     if (this.#position === -1) {
       if (this.#commands.length > 0) {
         // All the commands have been undone and then a new one is added
@@ -317,7 +319,9 @@ class CommandManager {
 
     // Avoid to insert something during the undo execution.
     this.#locked = true;
-    this.#commands[this.#position].undo();
+    const { undo, post } = this.#commands[this.#position];
+    undo();
+    post?.();
     this.#locked = false;
 
     this.#position -= 1;
@@ -332,7 +336,9 @@ class CommandManager {
 
       // Avoid to insert something during the redo execution.
       this.#locked = true;
-      this.#commands[this.#position].cmd();
+      const { cmd, post } = this.#commands[this.#position];
+      cmd();
+      post?.();
       this.#locked = false;
     }
   }
@@ -1442,6 +1448,24 @@ class AnnotationEditorUIManager {
     }
   }
 
+  get #lastSelectedEditor() {
+    let ed = null;
+    for (ed of this.#selectedEditors) {
+      // Iterate to get the last element.
+    }
+    return ed;
+  }
+
+  /**
+   * Update the UI of the active editor.
+   * @param {AnnotationEditor} editor
+   */
+  updateUI(editor) {
+    if (this.#lastSelectedEditor === editor) {
+      this.#dispatchUpdateUI(editor.propertiesToUpdate);
+    }
+  }
+
   /**
    * Add or remove an editor the current selection.
    * @param {AnnotationEditor} editor
@@ -1607,6 +1631,9 @@ class AnnotationEditorUIManager {
    * @param {Array<AnnotationEditor>} editors
    */
   #selectEditors(editors) {
+    for (const editor of this.#selectedEditors) {
+      editor.unselect();
+    }
     this.#selectedEditors.clear();
     for (const editor of editors) {
       if (editor.isEmpty()) {
@@ -1615,7 +1642,7 @@ class AnnotationEditorUIManager {
       this.#selectedEditors.add(editor);
       editor.select();
     }
-    this.#dispatchUpdateStates({ hasSelectedEditor: true });
+    this.#dispatchUpdateStates({ hasSelectedEditor: this.hasSelection });
   }
 
   /**

test/integration/freetext_editor_spec.mjs

@@ -866,7 +866,7 @@ describe("FreeText Editor", () => {
             .toEqual([13, 13]);
 
           // Change the colors for all the annotations.
-          page.evaluate(() => {
+          await page.evaluate(() => {
             window.PDFViewerApplication.eventBus.dispatch(
               "switchannotationeditorparams",
               {
@@ -3293,4 +3293,83 @@ describe("FreeText Editor", () => {
       );
     });
   });
+
+  describe("Freetext UI when undoing/redoing", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that the parameters are updated when undoing/redoing", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToFreeText(page);
+
+          const rect = await page.$eval(".annotationEditorLayer", el => {
+            // With Chrome something is wrong when serializing a DomRect,
+            // hence we extract the values and just return them.
+            const { x, y } = el.getBoundingClientRect();
+            return { x, y };
+          });
+
+          const data = "Hello PDF.js World !!";
+          await page.mouse.click(rect.x + 100, rect.y + 100);
+          await page.waitForSelector(getEditorSelector(0), {
+            visible: true,
+          });
+          await page.type(`${getEditorSelector(0)} .internal`, data);
+
+          // Commit.
+          await page.keyboard.press("Escape");
+          await page.waitForSelector(
+            `${getEditorSelector(0)} .overlay.enabled`
+          );
+
+          await page.evaluate(() => {
+            window.PDFViewerApplication.eventBus.dispatch(
+              "switchannotationeditorparams",
+              {
+                source: null,
+                type: window.pdfjsLib.AnnotationEditorParamsType.FREETEXT_COLOR,
+                value: "#FF0000",
+              }
+            );
+          });
+          await page.waitForFunction(
+            () =>
+              getComputedStyle(
+                document.querySelector(".selectedEditor .internal")
+              ).color === "rgb(255, 0, 0)"
+          );
+          await kbUndo(page);
+          await page.waitForFunction(
+            () =>
+              getComputedStyle(
+                document.querySelector(".selectedEditor .internal")
+              ).color === "rgb(0, 0, 0)"
+          );
+          await page.waitForFunction(
+            () =>
+              document.getElementById("editorFreeTextColor").value === "#000000"
+          );
+          await kbRedo(page);
+          await page.waitForFunction(
+            () =>
+              getComputedStyle(
+                document.querySelector(".selectedEditor .internal")
+              ).color === "rgb(255, 0, 0)"
+          );
+          await page.waitForFunction(
+            () =>
+              document.getElementById("editorFreeTextColor").value === "#ff0000"
+          );
+        })
+      );
+    });
+  });
 });