From 72df4b2ca2c62db14e99f541e146dc815ce4d20e Mon Sep 17 00:00:00 2001 From: tbolis Date: Mon, 2 Nov 2026 10:09:38 +0200 Subject: [PATCH] fix: the objects added while a drawing tool is in use are frozen The tools which draw a shape or pan freeze the objects of the canvas when they take over, but not the ones added afterwards: an image from addImg(), a text from addText(), a pasted object, and one which undo brings back were selectable. A press on an image with the arrow tool selected it, and a drag moved it instead of drawing the arrow. The tools get isInteractive(), and the events manager freezes what is added while the tool in use is not interactive. The select tool unfreezes all of them when it takes over, as before. --- CHANGELOG.md | 5 + README.md | 30 ++-- .../SketchField/addedObjects.spec.tsx | 164 ++++++++++++++++++ .../src/components/SketchField/events.ts | 6 + .../src/components/SketchField/tools/index.ts | 13 ++ 5 files changed, 203 insertions(+), 15 deletions(-) create mode 100644 packages/react-sketch/src/components/SketchField/addedObjects.spec.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ab7d23..eb401ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,11 @@ The eraser, laser pointer and polygon tools, pinch zoom, and features that forks - The arrow tool: an arrow is one undo step, and `onObjectAdded` is called once with it, and `onObjectRemoved` is not called. The line and the head which are shown while the arrow is drawn were recorded and announced too, so an arrow took three undos, and `onObjectAdded` was called three times. +- The tools which draw a shape or pan (line, arrow, rectangle, rectangle label, circle, polygon, pan and + the default tool) freeze the objects which are added while they are in use too: an image from `addImg()`, + a text from `addText()`, a pasted object, and one which undo brings back. They were selectable, so a press + on an image drew nothing and moved the image instead, with the arrow or any other of these tools. They are + selectable again when the select tool takes over. ### Packaging and tooling diff --git a/README.md b/README.md index d74fbbf..9be974f 100644 --- a/README.md +++ b/README.md @@ -292,21 +292,21 @@ throws. Use the tools by name (`"pencil"`), with `Tools` (`Tools.Pencil`) or with the enumeration `Types.Tool` (`Types.Tool.PENCIL`). -| Tool | `Tools` | Value | Description | -| --------------- | ---------------------- | ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Select | `Tools.Select` | `select` | Disables drawing. Objects can be selected, moved, scaled and rotated. | -| Pencil | `Tools.Pencil` | `pencil` | Free drawing. | -| Line | `Tools.Line` | `line` | Straight lines. | -| Arrow | `Tools.Arrow` | `arrow` | Lines with an arrow head. | -| Rectangle | `Tools.Rectangle` | `rectangle` | Rectangles, filled with `fillColor`. | -| Rectangle label | `Tools.RectangleLabel` | `rectangle-label` | Rectangles with a text box above them. The text box follows the rectangle when it is moved or scaled. | -| Polygon | `Tools.Polygon` | `polygon` | Closed shapes of straight sides, filled with `fillColor`. Each click or tap is a vertex. See [Polygon](#polygon). | -| Circle | `Tools.Circle` | `circle` | Circles, filled with `fillColor`. The drag from the first to the second point is the diameter. | -| Pan | `Tools.Pan` | `pan` | Disables drawing. Dragging moves the whole canvas, useful after zooming (contributed by [wmaillard](https://github.com/wmaillard)). | -| Highlighter | `Tools.Highlighter` | `highlighter` | Free drawing in a semi-transparent color. `lineColor` must be a hex code or a color name; any other value falls back to the default color. | -| Eraser | `Tools.Eraser` | `eraser` | Erases parts of the objects, with the width of `lineWidth`. The cursor shows the size of the brush. The background color and the background image are not erased. See [Eraser](#eraser). | -| Laser pointer | `Tools.LaserPointer` | `laser-pointer` | A line that follows the pointer, fades out and disappears. It is not part of the drawing. See [Laser pointer](#laser-pointer). | -| Default tool | `Tools.DefaultTool` | `default-tool` | Disables drawing and selection. Use it to show a drawing that is not edited. | +| Tool | `Tools` | Value | Description | +| --------------- | ---------------------- | ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Select | `Tools.Select` | `select` | Disables drawing. Objects can be selected, moved, scaled and rotated. The tools which draw a shape, and the pan tool, freeze the objects (also the ones added while they are in use), so a drag draws or pans. | +| Pencil | `Tools.Pencil` | `pencil` | Free drawing. | +| Line | `Tools.Line` | `line` | Straight lines. | +| Arrow | `Tools.Arrow` | `arrow` | Lines with an arrow head. | +| Rectangle | `Tools.Rectangle` | `rectangle` | Rectangles, filled with `fillColor`. | +| Rectangle label | `Tools.RectangleLabel` | `rectangle-label` | Rectangles with a text box above them. The text box follows the rectangle when it is moved or scaled. | +| Polygon | `Tools.Polygon` | `polygon` | Closed shapes of straight sides, filled with `fillColor`. Each click or tap is a vertex. See [Polygon](#polygon). | +| Circle | `Tools.Circle` | `circle` | Circles, filled with `fillColor`. The drag from the first to the second point is the diameter. | +| Pan | `Tools.Pan` | `pan` | Disables drawing. Dragging moves the whole canvas, useful after zooming (contributed by [wmaillard](https://github.com/wmaillard)). | +| Highlighter | `Tools.Highlighter` | `highlighter` | Free drawing in a semi-transparent color. `lineColor` must be a hex code or a color name; any other value falls back to the default color. | +| Eraser | `Tools.Eraser` | `eraser` | Erases parts of the objects, with the width of `lineWidth`. The cursor shows the size of the brush. The background color and the background image are not erased. See [Eraser](#eraser). | +| Laser pointer | `Tools.LaserPointer` | `laser-pointer` | A line that follows the pointer, fades out and disappears. It is not part of the drawing. See [Laser pointer](#laser-pointer). | +| Default tool | `Tools.DefaultTool` | `default-tool` | Disables drawing and selection. Use it to show a drawing that is not edited. | ### Line style diff --git a/packages/react-sketch/src/components/SketchField/addedObjects.spec.tsx b/packages/react-sketch/src/components/SketchField/addedObjects.spec.tsx new file mode 100644 index 0000000..9a3786a --- /dev/null +++ b/packages/react-sketch/src/components/SketchField/addedObjects.spec.tsx @@ -0,0 +1,164 @@ +import { createRef } from "react"; +import { act, render } from "@testing-library/react"; +import * as fabric from "fabric"; +import SketchField from "./SketchField"; +import { SketchProperties, Tool } from "../../types"; + +type Point = [number, number]; + +const mount = (props: SketchProperties = {}) => { + const ref = createRef(); + const view = render(); + const sketch = ref.current as SketchField; + return { + ...view, + sketch, + canvas: sketch["canvas"] as fabric.Canvas, + /** The same sketch, with another tool */ + use: (tool: Tool) => + view.rerender(), + }; +}; + +/** The image which `addImg` adds: jsdom loads no image */ +const stubImages = () => + jest.spyOn(fabric.FabricImage, "fromURL").mockImplementation(() => + Promise.resolve( + new fabric.FabricImage(document.createElement("img"), { + width: 100, + height: 60, + }) + ) + ); + +const settle = () => act(async () => void (await jest.advanceTimersByTimeAsync(50))); + +const mouseEvent = (type: string, [x, y]: Point): MouseEvent => + new MouseEvent(type, { + clientX: x, + clientY: y, + buttons: type === "mouseup" ? 0 : 1, + bubbles: true, + cancelable: true, + }); + +/** Press the mouse at `from`, drag it to `to` and release it, as the browser tells the canvas */ +const drag = (canvas: fabric.Canvas, from: Point, to: Point) => { + canvas.upperCanvasEl.dispatchEvent(mouseEvent("mousedown", from)); + document.dispatchEvent( + mouseEvent("mousemove", [(from[0] + to[0]) / 2, (from[1] + to[1]) / 2]) + ); + document.dispatchEvent(mouseEvent("mousemove", to)); + document.dispatchEvent(mouseEvent("mouseup", to)); +}; + +/** The tools which freeze the objects, as a drag with them always draws, or pans */ +const FROZEN = [ + Tool.LINE, + Tool.ARROW, + Tool.RECTANGLE, + Tool.RECTANGLE_LABEL, + Tool.CIRCLE, + Tool.POLYGON, + Tool.PAN, + Tool.DEFAULT_TOOL, +]; + +describe("objects added while a tool draws", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + describe.each(FROZEN)("with the %s tool", (tool) => { + it("are not selectable: an image, a text", async () => { + stubImages(); + const { sketch, canvas } = mount({ tool }); + + sketch.addImg("data:image/png;base64,00", { left: 10, top: 10, scale: 1 }); + sketch.addText("Hello"); + await settle(); + + const objects = canvas.getObjects(); + // The text is added at once, the image when it is loaded + expect(objects.map((o) => o.type).sort()).toEqual(["i-text", "image"]); + objects.forEach((object) => { + expect(object.selectable).toBe(false); + expect(object.evented).toBe(false); + }); + }); + }); + + it("are selectable with the select tool, and with the tools which draw free hand", async () => { + stubImages(); + for (const tool of [Tool.SELECT, Tool.PENCIL, Tool.HIGHLIGHTER, Tool.ERASER]) { + const { sketch, canvas, unmount } = mount({ tool }); + + sketch.addImg("data:image/png;base64,00"); + sketch.addText("Hello"); + await settle(); + + canvas.getObjects().forEach((object) => { + expect(object.selectable).toBe(true); + expect(object.evented).toBe(true); + }); + unmount(); + } + }); + + it("are selectable again when the select tool takes over", async () => { + stubImages(); + const { sketch, canvas, use } = mount({ tool: Tool.ARROW }); + sketch.addImg("data:image/png;base64,00"); + await settle(); + const [image] = canvas.getObjects(); + expect(image.selectable).toBe(false); + + use(Tool.SELECT); + + expect(image.selectable).toBe(true); + expect(image.evented).toBe(true); + }); + + it("do not move when an arrow is drawn from them, and the arrow is drawn", async () => { + stubImages(); + const { sketch, canvas } = mount({ tool: Tool.ARROW }); + sketch.addImg("data:image/png;base64,00", { left: 100, top: 100, scale: 1 }); + await settle(); + const [image] = canvas.getObjects(); + + drag(canvas, [120, 120], [300, 200]); + + expect(canvas.getActiveObject()).toBeUndefined(); + expect([image.left, image.top]).toEqual([100, 100]); + const objects = canvas.getObjects(); + expect(objects).toHaveLength(2); + expect(objects[1]).toBeInstanceOf(fabric.Group); + }); + + it("are not selectable when a paste adds them, or an undo brings them back", async () => { + const { sketch, canvas, use } = mount({ tool: Tool.SELECT }); + const rect = new fabric.Rect({ left: 10, top: 10, width: 50, height: 50 }); + canvas.add(rect); + canvas.setActiveObject(rect); + sketch.copy(); + sketch.removeSelected(); + expect(canvas.getObjects()).toHaveLength(0); + + use(Tool.ARROW); + sketch.undo(); + sketch.paste(); + await settle(); + + const objects = canvas.getObjects(); + expect(objects).toHaveLength(2); + objects.forEach((object) => { + expect(object.selectable).toBe(false); + expect(object.evented).toBe(false); + }); + }); +}); diff --git a/packages/react-sketch/src/components/SketchField/events.ts b/packages/react-sketch/src/components/SketchField/events.ts index 7e42de9..37a8c71 100644 --- a/packages/react-sketch/src/components/SketchField/events.ts +++ b/packages/react-sketch/src/components/SketchField/events.ts @@ -76,6 +76,12 @@ export class FabricCanvasEventsManager */ public onObjectAdded({ event, tool, props }: EventInput): void { if (this.isPreview(event, tool)) return; + // What is added while a tool draws (an image, a text, a pasted object, one which + // undo brings back) is frozen like the rest: a press on it would select it and + // a drag would move it, instead of drawing + if (event.target && !tool.isInteractive()) { + event.target.set({ selectable: false, evented: false }); + } // Only erasable objects are affected by the eraser if (event.target) (event.target as { erasable?: boolean }).erasable = true; if (this.history.replaying) return; diff --git a/packages/react-sketch/src/components/SketchField/tools/index.ts b/packages/react-sketch/src/components/SketchField/tools/index.ts index a5928d9..b7fa1db 100644 --- a/packages/react-sketch/src/components/SketchField/tools/index.ts +++ b/packages/react-sketch/src/components/SketchField/tools/index.ts @@ -69,6 +69,7 @@ export abstract class AbstractEventAware implements CanvasEventAware { */ export abstract class FabricCanvasTool { private readonly _canvas: fabric.Canvas; + private interactive = true; protected constructor(canvas: fabric.Canvas) { this._canvas = canvas; @@ -98,6 +99,17 @@ export abstract class FabricCanvasTool { */ dispose(): void {} + /** + * Whether the objects can be selected and moved with the tool. It is not for + * the tools which freeze them, so a press on an object draws instead of + * moving it: the objects which are added while the tool is in use are frozen + * too. The tools which draw free hand leave them as they are, as fabric + * does not select an object in drawing mode. + */ + isInteractive(): boolean { + return this.interactive; + } + /** * Called when the content of the canvas is replaced (it is cleared, a value * is loaded, an undo or a redo is done), for the tools which hold something @@ -178,6 +190,7 @@ export abstract class FabricCanvasTool { * Free drawing is turned off in both. */ protected setInteractive(interactive: boolean): void { + this.interactive = interactive; this.canvas.isDrawingMode = false; this.canvas.selection = interactive; this.canvas.forEachObject((o) => (o.selectable = o.evented = interactive));