Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions src/display/editor/freetext.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,9 @@ class FreeTextEditor extends AnnotationEditor {
};
const savedFontsize = this.#fontSize;
this.addCommands({
cmd: () => {
setFontsize(fontSize);
},
undo: () => {
setFontsize(savedFontsize);
},
cmd: setFontsize.bind(this, fontSize),
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
undo: setFontsize.bind(this, savedFontsize),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.FREETEXT_SIZE,
overwriteIfSameType: true,
Expand All @@ -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: () => {
this.#color = this.editorDiv.style.color = color;
},
undo: () => {
this.#color = this.editorDiv.style.color = savedColor;
},
cmd: setColor.bind(this, color),
undo: setColor.bind(this, savedColor),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.FREETEXT_COLOR,
overwriteIfSameType: true,
Expand Down
18 changes: 8 additions & 10 deletions src/display/editor/highlight.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {
this.color = color;
this.parent?.drawLayer.changeColor(this.#id, color);
this.#colorPicker?.updateColor(color);
},
undo: () => {
this.color = savedColor;
this.parent?.drawLayer.changeColor(this.#id, savedColor);
this.#colorPicker?.updateColor(savedColor);
},
cmd: setColor.bind(this, color),
undo: setColor.bind(this, savedColor),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.HIGHLIGHT_COLOR,
overwriteIfSameType: true,
Expand Down
45 changes: 21 additions & 24 deletions src/display/editor/ink.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {
this.thickness = thickness;
this.#fitToContent();
},
undo: () => {
this.thickness = savedThickness;
this.#fitToContent();
},
cmd: setThickness.bind(this, thickness),
undo: setThickness.bind(this, savedThickness),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.INK_THICKNESS,
overwriteIfSameType: true,
Expand All @@ -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: () => {
this.color = color;
this.#redraw();
},
undo: () => {
this.color = savedColor;
this.#redraw();
},
cmd: setColor.bind(this, color),
undo: setColor.bind(this, savedColor),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.INK_COLOR,
overwriteIfSameType: true,
Expand All @@ -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: () => {
this.opacity = opacity;
this.#redraw();
},
undo: () => {
this.opacity = savedOpacity;
this.#redraw();
},
cmd: setOpacity.bind(this, opacity),
undo: setOpacity.bind(this, savedOpacity),
post: this._uiManager.updateUI.bind(this._uiManager, this),
mustExec: true,
type: AnnotationEditorParamsType.INK_OPACITY,
overwriteIfSameType: true,
Expand Down
35 changes: 31 additions & 4 deletions src/display/editor/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -257,6 +258,7 @@ class CommandManager {
add({
cmd,
undo,
post,
mustExec,
type = NaN,
overwriteIfSameType = false,
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -1615,7 +1642,7 @@ class AnnotationEditorUIManager {
this.#selectedEditors.add(editor);
editor.select();
}
this.#dispatchUpdateStates({ hasSelectedEditor: true });
this.#dispatchUpdateStates({ hasSelectedEditor: this.hasSelection });
}

/**
Expand Down
81 changes: 80 additions & 1 deletion test/integration/freetext_editor_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
{
Expand Down Expand Up @@ -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"
);
})
);
});
});
});