diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index d41d66c1f..6edee9234 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -18,8 +18,13 @@ // eslint-disable-next-line max-len /** @typedef {import("./tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */ +import { + AnnotationEditorParamsType, + FeatureTest, + shadow, + unreachable, +} from "../../shared/util.js"; import { bindEvents, ColorManager } from "./tools.js"; -import { FeatureTest, shadow, unreachable } from "../../shared/util.js"; /** * @typedef {Object} AnnotationEditorParameters @@ -261,13 +266,7 @@ class AnnotationEditor { this.fixAndSetPosition(); } - /** - * Translate the editor position within its parent. - * @param {number} x - x-translation in screen coordinates. - * @param {number} y - y-translation in screen coordinates. - */ - translate(x, y) { - const [width, height] = this.parentDimensions; + #translate([width, height], x, y) { [x, y] = this.screenToPageTranslation(x, y); this.x += x / width; @@ -276,6 +275,26 @@ class AnnotationEditor { this.fixAndSetPosition(); } + /** + * Translate the editor position within its parent. + * @param {number} x - x-translation in screen coordinates. + * @param {number} y - y-translation in screen coordinates. + */ + translate(x, y) { + this.#translate(this.parentDimensions, x, y); + } + + /** + * Translate the editor position within its page and adjust the scroll + * in order to have the editor in the view. + * @param {number} x - x-translation in page coordinates. + * @param {number} y - y-translation in page coordinates. + */ + translateInPage(x, y) { + this.#translate(this.pageDimensions, x, y); + this.div.scrollIntoView({ block: "nearest" }); + } + fixAndSetPosition() { const [pageWidth, pageHeight] = this.pageDimensions; let { x, y, width, height } = this; @@ -663,7 +682,7 @@ class AnnotationEditor { cmd, undo, mustExec: true, - type: this.resizeType, + type: AnnotationEditorParamsType.RESIZE, overwriteIfSameType: true, keepUndo: true, }); @@ -922,13 +941,6 @@ class AnnotationEditor { } } - /** - * @returns {number} the type to use in the undo/redo stack when resizing. - */ - get resizeType() { - return -1; - } - /** * @returns {boolean} true if this editor can be resized. */ diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index cc20bf45a..8baf88ddf 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -71,7 +71,7 @@ class FreeTextEditor extends AnnotationEditor { // See bug 1831574. ["ctrl+s", "mac+meta+s", "ctrl+p", "mac+meta+p"], FreeTextEditor.prototype.commitOrRemove, - /* bubbles = */ true, + { bubbles: true }, ], [ ["ctrl+Enter", "mac+meta+Enter", "Escape", "mac+Escape"], diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js index 471fd04a8..8c69dcd46 100644 --- a/src/display/editor/ink.js +++ b/src/display/editor/ink.js @@ -157,11 +157,6 @@ class InkEditor extends AnnotationEditor { ]; } - /** @inheritdoc */ - get resizeType() { - return AnnotationEditorParamsType.INK_DIMS; - } - /** * Update the thickness and make this action undoable. * @param {number} thickness diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index 8c26d0ce6..8dac89005 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -13,11 +13,8 @@ * limitations under the License. */ -import { - AnnotationEditorParamsType, - AnnotationEditorType, -} from "../../shared/util.js"; import { AnnotationEditor } from "./editor.js"; +import { AnnotationEditorType } from "../../shared/util.js"; import { PixelsPerInch } from "../display_utils.js"; import { StampAnnotationElement } from "../annotation_layer.js"; @@ -126,11 +123,6 @@ class StampEditor extends AnnotationEditor { } } - /** @inheritdoc */ - get resizeType() { - return AnnotationEditorParamsType.STAMP_DIMS; - } - /** @inheritdoc */ remove() { if (this.#bitmapId) { diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index e72935f8b..99ed5651f 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -355,14 +355,14 @@ class KeyboardManager { this.allKeys = new Set(); const { isMac } = FeatureTest.platform; - for (const [keys, callback, bubbles = false] of callbacks) { + for (const [keys, callback, options = {}] of callbacks) { for (const key of keys) { const isMacKey = key.startsWith("mac+"); if (isMac && isMacKey) { - this.callbacks.set(key.slice(4), { callback, bubbles }); + this.callbacks.set(key.slice(4), { callback, options }); this.allKeys.add(key.split("+").at(-1)); } else if (!isMac && !isMacKey) { - this.callbacks.set(key, { callback, bubbles }); + this.callbacks.set(key, { callback, options }); this.allKeys.add(key.split("+").at(-1)); } } @@ -410,8 +410,15 @@ class KeyboardManager { if (!info) { return; } - const { callback, bubbles } = info; - callback.bind(self)(); + const { + callback, + options: { bubbles = false, args = [], checker = null }, + } = info; + + if (checker && !checker(self, event)) { + return; + } + callback.bind(self, ...args)(); // For example, ctrl+s in a FreeText must be handled by the viewer, hence // the event must bubble. @@ -548,9 +555,29 @@ class AnnotationEditorUIManager { hasSelectedEditor: false, }; + #translation = [0, 0]; + + #translationTimeoutId = null; + #container = null; + static #TRANSLATE_SMALL = 1; // page units. + + static #TRANSLATE_BIG = 10; // page units. + static get _keyboardManager() { + const arrowChecker = self => { + // If the focused element is an input, we don't want to handle the arrow. + // For example, sliders can be controlled with the arrow keys. + const { activeElement } = document; + return ( + activeElement && + self.#container.contains(activeElement) && + self.hasSomethingToControl() + ); + }; + const small = this.#TRANSLATE_SMALL; + const big = this.#TRANSLATE_BIG; return shadow( this, "_keyboardManager", @@ -592,6 +619,46 @@ class AnnotationEditorUIManager { ["Escape", "mac+Escape"], AnnotationEditorUIManager.prototype.unselectAll, ], + [ + ["ArrowLeft", "mac+ArrowLeft"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [-small, 0], checker: arrowChecker }, + ], + [ + ["ctrl+ArrowLeft", "mac+meta+ArrowLeft"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [-big, 0], checker: arrowChecker }, + ], + [ + ["ArrowRight", "mac+ArrowRight"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [small, 0], checker: arrowChecker }, + ], + [ + ["ctrl+ArrowRight", "mac+meta+ArrowRight"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [big, 0], checker: arrowChecker }, + ], + [ + ["ArrowUp", "mac+ArrowUp"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [0, -small], checker: arrowChecker }, + ], + [ + ["ctrl+ArrowUp", "mac+meta+ArrowUp"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [0, -big], checker: arrowChecker }, + ], + [ + ["ArrowDown", "mac+ArrowDown"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [0, small], checker: arrowChecker }, + ], + [ + ["ctrl+ArrowDown", "mac+meta+ArrowDown"], + AnnotationEditorUIManager.prototype.translateSelectedEditors, + { args: [0, big], checker: arrowChecker }, + ], ]) ); } @@ -1260,6 +1327,10 @@ class AnnotationEditorUIManager { this.#activeEditor?.commitOrRemove(); } + hasSomethingToControl() { + return this.#activeEditor || this.hasSelection; + } + /** * Select the editors. * @param {Array} editors @@ -1296,7 +1367,7 @@ class AnnotationEditorUIManager { return; } - if (this.#selectedEditors.size === 0) { + if (!this.hasSelection) { return; } for (const editor of this.#selectedEditors) { @@ -1308,6 +1379,53 @@ class AnnotationEditorUIManager { }); } + translateSelectedEditors(x, y) { + this.commitOrRemove(); + if (!this.hasSelection) { + return; + } + + this.#translation[0] += x; + this.#translation[1] += y; + const [totalX, totalY] = this.#translation; + const editors = [...this.#selectedEditors]; + + // We don't want to have an undo/redo for each translation so we wait a bit + // before adding the command to the command manager. + const TIME_TO_WAIT = 1000; + + if (this.#translationTimeoutId) { + clearTimeout(this.#translationTimeoutId); + } + + this.#translationTimeoutId = setTimeout(() => { + this.#translationTimeoutId = null; + this.#translation[0] = this.#translation[1] = 0; + + this.addCommands({ + cmd: () => { + for (const editor of editors) { + if (this.#allEditors.has(editor.id)) { + editor.translateInPage(totalX, totalY); + } + } + }, + undo: () => { + for (const editor of editors) { + if (this.#allEditors.has(editor.id)) { + editor.translateInPage(-totalX, -totalY); + } + } + }, + mustExec: false, + }); + }, TIME_TO_WAIT); + + for (const editor of editors) { + editor.translateInPage(x, y); + } + } + /** * Is the current editor the one passed as argument? * @param {AnnotationEditor} editor diff --git a/src/shared/util.js b/src/shared/util.js index 3b5ea5614..d5359505c 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -77,14 +77,13 @@ const AnnotationEditorType = { }; const AnnotationEditorParamsType = { - FREETEXT_SIZE: 1, - FREETEXT_COLOR: 2, - FREETEXT_OPACITY: 3, - INK_COLOR: 11, - INK_THICKNESS: 12, - INK_OPACITY: 13, - INK_DIMS: 14, - STAMP_DIMS: 21, + RESIZE: 1, + FREETEXT_SIZE: 11, + FREETEXT_COLOR: 12, + FREETEXT_OPACITY: 13, + INK_COLOR: 21, + INK_THICKNESS: 22, + INK_OPACITY: 23, }; // Permission flags from Table 22, Section 7.6.3.2 of the PDF specification. diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index ab5c6ad0b..4f4650480 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -18,6 +18,7 @@ const { getEditors, getEditorSelector, getSelectedEditors, + getFirstSerialized, getSerialized, loadAndWait, waitForEvent, @@ -752,7 +753,7 @@ describe("FreeText Editor", () => { "switchannotationeditorparams", { source: null, - type: /* AnnotationEditorParamsType.FREETEXT_SIZE */ 1, + type: window.pdfjsLib.AnnotationEditorParamsType.FREETEXT_SIZE, value: 13, } ); @@ -769,7 +770,7 @@ describe("FreeText Editor", () => { "switchannotationeditorparams", { source: null, - type: /* AnnotationEditorParamsType.FREETEXT_COLOR */ 2, + type: window.pdfjsLib.AnnotationEditorParamsType.FREETEXT_COLOR, value: "#FF0000", } ); @@ -1310,7 +1311,7 @@ describe("FreeText Editor", () => { "switchannotationeditorparams", { source: null, - type: /* AnnotationEditorParamsType.FREETEXT_SIZE */ 1, + type: window.pdfjsLib.AnnotationEditorParamsType.FREETEXT_SIZE, value: 50, } ); @@ -1780,4 +1781,142 @@ describe("FreeText Editor", () => { ); }); }); + + describe("Move editor with arrows", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("empty.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must check the position of moved editor", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorFreeText"); + + const rect = await page.$eval(".annotationEditorLayer", el => { + const { x, y } = el.getBoundingClientRect(); + return { x, y }; + }); + + const data = "Hello PDF.js World !!"; + await page.mouse.click(rect.x + 200, rect.y + 200); + await page.type(`${getEditorSelector(0)} .internal`, data); + + const editorRect = await page.$eval(getEditorSelector(0), el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { + x, + y, + width, + height, + }; + }); + + // Commit. + await page.mouse.click( + editorRect.x, + editorRect.y + 2 * editorRect.height + ); + await page.waitForTimeout(10); + + const [pageX, pageY] = await getFirstSerialized(page, x => x.rect); + + for (let i = 0; i < 20; i++) { + await page.keyboard.press("ArrowRight"); + await page.waitForTimeout(1); + } + + let [newX, newY] = await getFirstSerialized(page, x => x.rect); + expect(Math.round(newX)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageX + 20)); + expect(Math.round(newY)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageY)); + + for (let i = 0; i < 20; i++) { + await page.keyboard.press("ArrowDown"); + await page.waitForTimeout(1); + } + + [newX, newY] = await getFirstSerialized(page, x => x.rect); + expect(Math.round(newX)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageX + 20)); + expect(Math.round(newY)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageY - 20)); + + for (let i = 0; i < 2; i++) { + await page.keyboard.down("Control"); + await page.keyboard.press("ArrowLeft"); + await page.keyboard.up("Control"); + await page.waitForTimeout(1); + } + + [newX, newY] = await getFirstSerialized(page, x => x.rect); + expect(Math.round(newX)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageX)); + expect(Math.round(newY)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageY - 20)); + + for (let i = 0; i < 2; i++) { + await page.keyboard.down("Control"); + await page.keyboard.press("ArrowUp"); + await page.keyboard.up("Control"); + await page.waitForTimeout(1); + } + + [newX, newY] = await getFirstSerialized(page, x => x.rect); + expect(Math.round(newX)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageX)); + expect(Math.round(newY)) + .withContext(`In ${browserName}`) + .toEqual(Math.round(pageY)); + }) + ); + }); + + it("must check arrow doesn't move an editor when a slider is focused", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.keyboard.down("Control"); + await page.keyboard.press("a"); + await page.keyboard.up("Control"); + await page.waitForTimeout(10); + + await page.focus("#editorFreeTextFontSize"); + + const [page1X, , page2X] = await getFirstSerialized( + page, + x => x.rect + ); + const pageWidth = page2X - page1X; + + for (let i = 0; i < 5; i++) { + await page.keyboard.press("ArrowRight"); + await page.waitForTimeout(1); + } + await page.waitForTimeout(10); + + const [new1X, , new2X] = await getFirstSerialized(page, x => x.rect); + const newWidth = new2X - new1X; + expect(Math.round(new1X)) + .withContext(`In ${browserName}`) + .not.toEqual(Math.round(page1X + 5)); + expect(newWidth) + .withContext(`In ${browserName}`) + .not.toEqual(pageWidth); + }) + ); + }); + }); }); diff --git a/test/integration/test_utils.js b/test/integration/test_utils.js index 08a3a1002..e6642b851 100644 --- a/test/integration/test_utils.js +++ b/test/integration/test_utils.js @@ -137,14 +137,20 @@ const mockClipboard = async pages => { }; exports.mockClipboard = mockClipboard; -const getSerialized = page => - page.evaluate(() => { +async function getSerialized(page, filter = undefined) { + const values = await page.evaluate(() => { const { map } = window.PDFViewerApplication.pdfDocument.annotationStorage.serializable; return map ? [...map.values()] : []; }); + return filter ? values.map(filter) : values; +} exports.getSerialized = getSerialized; +const getFirstSerialized = async (page, filter = undefined) => + (await getSerialized(page, filter))[0]; +exports.getFirstSerialized = getFirstSerialized; + function getEditors(page, kind) { return page.evaluate(aKind => { const elements = document.querySelectorAll(`.${aKind}Editor`);