Skip to content

Commit

Permalink
Merge pull request #15242 from calixteman/ink_avoid_move
Browse files Browse the repository at this point in the history
[Editor] Avoid to slightly move ink editor when undoing/redoing
  • Loading branch information
Snuffleupagus authored Jul 29, 2022
2 parents 6bb800a + 9a464b7 commit cfdf940
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 7 deletions.
12 changes: 5 additions & 7 deletions src/display/editor/ink.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ class InkEditor extends AnnotationEditor {
this.parent.addCommands({
cmd: () => {
this.thickness = thickness;
this.#fitToContent(/* thicknessChanged = */ true);
this.#fitToContent();
},
undo: () => {
this.thickness = savedThickness;
this.#fitToContent(/* thicknessChanged = */ true);
this.#fitToContent();
},
mustExec: true,
type: AnnotationEditorParamsType.INK_THICKNESS,
Expand Down Expand Up @@ -478,7 +478,7 @@ class InkEditor extends AnnotationEditor {
this.#disableEditing = true;
this.div.classList.add("disabled");

this.#fitToContent();
this.#fitToContent(/* firstTime = */ true);

this.parent.addInkEditorIfNeeded(/* isCommitting = */ true);

Expand Down Expand Up @@ -928,7 +928,7 @@ class InkEditor extends AnnotationEditor {
* the bounding box of the contents.
* @returns {undefined}
*/
#fitToContent(thicknessChanged = false) {
#fitToContent(firstTime = false) {
if (this.isEmpty()) {
return;
}
Expand Down Expand Up @@ -965,9 +965,7 @@ class InkEditor extends AnnotationEditor {
this.#realHeight = height;

this.setDims(width, height);
const unscaledPadding = thicknessChanged
? 0
: padding / this.scaleFactor / 2;
const unscaledPadding = firstTime ? padding / this.scaleFactor / 2 : 0;
this.translate(
prevTranslationX - this.translationX - unscaledPadding,
prevTranslationY - this.translationY - unscaledPadding
Expand Down
60 changes: 60 additions & 0 deletions test/integration/ink_editor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,65 @@ describe("Editor", () => {
})
);
});

it("must draw, undo/redo and check that the editor don't move", 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.keyboard.press("Backspace");

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 xStart = rect.x + 300;
const yStart = rect.y + 300;
await page.mouse.move(xStart, yStart);
await page.mouse.down();
await page.mouse.move(xStart + 50, yStart + 50);
await page.mouse.up();

await page.keyboard.press("Escape");

const rectBefore = await page.$eval(".inkEditor canvas", el => {
const { x, y } = el.getBoundingClientRect();
return { x, y };
});

for (let i = 0; i < 30; i++) {
await page.keyboard.down("Control");
await page.keyboard.press("z");
await page.keyboard.up("Control");

await page.waitForTimeout(10);

await page.keyboard.down("Control");
await page.keyboard.press("y");
await page.keyboard.up("Control");

await page.waitForTimeout(10);
}

const rectAfter = await page.$eval(".inkEditor canvas", el => {
const { x, y } = el.getBoundingClientRect();
return { x, y };
});

expect(Math.round(rectBefore.x))
.withContext(`In ${browserName}`)
.toEqual(Math.round(rectAfter.x));

expect(Math.round(rectBefore.y))
.withContext(`In ${browserName}`)
.toEqual(Math.round(rectAfter.y));
})
);
});
});
});

0 comments on commit cfdf940

Please sign in to comment.