diff --git a/CHANGELOG.md b/CHANGELOG.md index df4a586..ae5d7c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,10 @@ The eraser, laser pointer and polygon tools, pinch zoom, and features that forks 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. +- The rectangle label tool: the canvas has two listeners for all the labels, not two more for each label + that is drawn. They were never removed, so they stayed after a label was deleted or the canvas was + cleared, and a canvas that is drawn on for long kept more of them. A label still follows its rectangle + when undo and redo take it off the canvas and put it back. ### Packaging and tooling diff --git a/packages/react-sketch/src/components/SketchField/tools/RectangleLabel/RectangleLabelObject.ts b/packages/react-sketch/src/components/SketchField/tools/RectangleLabel/RectangleLabelObject.ts index a1db611..162f1b5 100644 --- a/packages/react-sketch/src/components/SketchField/tools/RectangleLabel/RectangleLabelObject.ts +++ b/packages/react-sketch/src/components/SketchField/tools/RectangleLabel/RectangleLabelObject.ts @@ -1,6 +1,27 @@ import * as fabric from "fabric"; import { CanvasEvent } from "../../events"; +/** The label of each rectangle, so that the listeners of a canvas serve all of its labels */ +const labels = new WeakMap(); + +/** The canvases which already have those listeners */ +const following = new WeakSet(); + +/** + * Make the labels of the canvas follow their rectangles. A label stays with its + * rectangle when it is taken off the canvas and put back (undo, redo), so the + * listeners are not removed with it: there are two for each canvas, not for each label. + */ +const follow = (canvas: fabric.Canvas): void => { + if (following.has(canvas)) return; + following.add(canvas); + const update = ({ target }: CanvasEvent): void => { + if (target) labels.get(target)?.update(); + }; + canvas.on("object:scaling", update); + canvas.on("object:moving", update); +}; + /** * A rectangle with a text box on top of it. The text box follows the * rectangle when it is moved or scaled. @@ -17,12 +38,11 @@ export class RectangleLabelObject { ) { this.rect = new fabric.Rect(rectProps); this.textbox = new fabric.Textbox(text, textProps); - canvas.on("object:scaling", this.update); - canvas.on("object:moving", this.update); + labels.set(this.rect, this); + follow(canvas); } - update = (event: CanvasEvent): void => { - if (event.target !== this.rect) return; + update(): void { this.textbox.set({ width: this.rect.getScaledWidth(), scaleX: 1, @@ -30,7 +50,7 @@ export class RectangleLabelObject { top: (this.rect.top ?? 0) - this.textbox.getScaledHeight(), left: this.rect.left, }); - }; + } setText(text: string): void { this.textbox.set({ text }); diff --git a/packages/react-sketch/src/components/SketchField/tools/tools.spec.ts b/packages/react-sketch/src/components/SketchField/tools/tools.spec.ts index d2d7654..6ed6d32 100644 --- a/packages/react-sketch/src/components/SketchField/tools/tools.spec.ts +++ b/packages/react-sketch/src/components/SketchField/tools/tools.spec.ts @@ -215,6 +215,68 @@ describe("Tools", () => { expect(text.left).toEqual(200); expect(text.top).toEqual(150 - text.getScaledHeight()); }); + + it("listens to the canvas once, whatever the number of labels", () => { + const on = jest.spyOn(canvas, "on"); + const tool = create_tool(canvas, { tool: Tool.RECTANGLE_LABEL }); + [100, 200, 300].forEach((y) => drag(tool, [100, y], [180, y + 60])); + + const listened = (name: string) => + on.mock.calls.filter(([event]) => event === name); + expect(canvas.getObjects()).toHaveLength(6); + expect(listened("object:moving")).toHaveLength(1); + expect(listened("object:scaling")).toHaveLength(1); + }); + + it("moves each label along with its own rectangle", () => { + const tool = create_tool(canvas, { tool: Tool.RECTANGLE_LABEL }); + drag(tool, [100, 100], [180, 160]); + drag(tool, [100, 300], [180, 360]); + + const [, first, second, other] = canvas.getObjects() as fabric.FabricObject[]; + const firstTop = first.top; + second.set({ left: 250, top: 320 }); + fire(canvas, "object:moving", { target: second }); + + expect(other.left).toEqual(250); + expect(other.top).toEqual(320 - other.getScaledHeight()); + expect(first.top).toEqual(firstTop); + }); + + it("moves the label of a canvas which is not the first one", () => { + const another = new fabric.Canvas(document.createElement("canvas")); + try { + drag( + create_tool(canvas, { tool: Tool.RECTANGLE_LABEL }), + [10, 10], + [50, 50] + ); + const tool = create_tool(another, { tool: Tool.RECTANGLE_LABEL }); + drag(tool, [100, 100], [180, 160]); + + const [rect, text] = another.getObjects() as fabric.FabricObject[]; + rect.set({ left: 200, top: 150 }); + fire(another, "object:moving", { target: rect }); + + expect(text.left).toEqual(200); + expect(text.top).toEqual(150 - text.getScaledHeight()); + } finally { + another.dispose(); + } + }); + + it("moves the label of a rectangle which undo and redo took off and put back", () => { + const tool = create_tool(canvas, { tool: Tool.RECTANGLE_LABEL }); + drag(tool, [100, 100], [180, 160]); + + const [rect, text] = canvas.getObjects() as fabric.FabricObject[]; + canvas.remove(rect); + canvas.add(rect); + rect.set({ left: 200, top: 150 }); + fire(canvas, "object:moving", { target: rect }); + + expect(text.left).toEqual(200); + }); }); describe("Pan", () => {