From 864149625d6a19fead4fb61605f3eb586c606295 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sat, 9 Sep 2023 19:52:10 +0530 Subject: [PATCH 1/8] patch(ActiveSelection): init logic --- src/canvas/SelectableCanvas.ts | 1 + src/shapes/ActiveSelection.ts | 10 ---------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index b01506c6b3b..d9a783d63cf 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -302,6 +302,7 @@ export class SelectableCanvas super(el, options); this._activeSelection = activeSelection; this._activeSelection.set('canvas', this); + this._activeSelection.setCoords(); } protected initElements(el: string | HTMLCanvasElement) { diff --git a/src/shapes/ActiveSelection.ts b/src/shapes/ActiveSelection.ts index 835c4e0ffea..be48541f531 100644 --- a/src/shapes/ActiveSelection.ts +++ b/src/shapes/ActiveSelection.ts @@ -3,7 +3,6 @@ import { classRegistry } from '../ClassRegistry'; import type { GroupProps, LayoutContext } from './Group'; import { Group } from './Group'; import type { FabricObject } from './Object/FabricObject'; -import type { TOptions } from '../typedefs'; export type MultiSelectionStacking = 'canvas-stacking' | 'selection-order'; @@ -26,15 +25,6 @@ export class ActiveSelection extends Group { static type = 'ActiveSelection'; - constructor( - objects?: FabricObject[], - options?: TOptions, - objectsRelativeToGroup?: boolean - ) { - super(objects, options, objectsRelativeToGroup); - this.setCoords(); - } - /** * @private */ From 30b5bfb5ab6035b46b48ee0a9e44faebc45dea17 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 9 Sep 2023 14:25:35 +0000 Subject: [PATCH 2/8] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b558fb957cc..a697ec63491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- patch(ActiveSelection): init logic [#9322](https://github.com/fabricjs/fabric.js/pull/9322) - docs(): add link to contributing guide [#8393](https://github.com/fabricjs/fabric.js/pull/8393) - test(e2e): Drag&Drop tests [#9112](https://github.com/fabricjs/fabric.js/pull/9112) - fix(CanvasEvents): regression of `getPointer` usages + BREAKING: drop event data [#9186](https://github.com/fabricjs/fabric.js/pull/9186) From 828b2d7d6252493b5342a0cda4518c04b1a6022e Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sat, 9 Sep 2023 20:04:26 +0530 Subject: [PATCH 3/8] test --- src/shapes/ActiveSelection.spec.ts | 17 ++++++++ .../ActiveSelection.spec.ts.snap | 43 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/shapes/__snapshots__/ActiveSelection.spec.ts.snap diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index f275d403138..e5469c4b3ae 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -1,3 +1,4 @@ +import { Canvas } from '../canvas/Canvas'; import { ActiveSelection } from './ActiveSelection'; import { FabricObject } from './Object/FabricObject'; @@ -55,4 +56,20 @@ describe('ActiveSelection', () => { selection.add(new FabricObject({ left: 50, top: 50, strokeWidth: 0 })); expect(selection.item(0).getCenterPoint()).toEqual({ x: 50, y: 50 }); }); + + it('sets coords after attaching to canvas', () => { + const canvas = new Canvas(null, { + activeSelection: new ActiveSelection([ + new FabricObject({ + left: 100, + top: 100, + width: 100, + height: 100, + }), + ]), + viewportTransform: [2, 0, 0, 0.5, 400, 150], + }); + expect(canvas.getActiveSelection().getCoords()).toMatchSnapshot(); + expect(canvas.getActiveSelection().getCoords(true)).toMatchSnapshot(); + }); }); diff --git a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap new file mode 100644 index 00000000000..b0c4157704f --- /dev/null +++ b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap @@ -0,0 +1,43 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ActiveSelection adding to canvas sets coords 1`] = ` +[ + Point { + "x": 600, + "y": 200, + }, + Point { + "x": 802, + "y": 200, + }, + Point { + "x": 802, + "y": 250.5, + }, + Point { + "x": 600, + "y": 250.5, + }, +] +`; + +exports[`ActiveSelection adding to canvas sets coords 2`] = ` +[ + Point { + "x": 100, + "y": 100, + }, + Point { + "x": 201, + "y": 100, + }, + Point { + "x": 201, + "y": 201, + }, + Point { + "x": 100, + "y": 201, + }, +] +`; From e23ecbdacc1436f0c889cd35508beea92b9fd18c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 9 Sep 2023 14:35:52 +0000 Subject: [PATCH 4/8] update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a697ec63491..1dea5ab6a56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [next] -- patch(ActiveSelection): init logic [#9322](https://github.com/fabricjs/fabric.js/pull/9322) +- fix(ActiveSelection): initial coords [#9322](https://github.com/fabricjs/fabric.js/pull/9322) - docs(): add link to contributing guide [#8393](https://github.com/fabricjs/fabric.js/pull/8393) - test(e2e): Drag&Drop tests [#9112](https://github.com/fabricjs/fabric.js/pull/9112) - fix(CanvasEvents): regression of `getPointer` usages + BREAKING: drop event data [#9186](https://github.com/fabricjs/fabric.js/pull/9186) From 14901f6f1282772a1f8c22387f7356caddaa9ca3 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sat, 9 Sep 2023 20:07:48 +0530 Subject: [PATCH 5/8] Update ActiveSelection.spec.ts.snap --- src/shapes/__snapshots__/ActiveSelection.spec.ts.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap index b0c4157704f..101e1d18ee8 100644 --- a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap +++ b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ActiveSelection adding to canvas sets coords 1`] = ` +exports[`ActiveSelection sets coords after attaching to canvas 1`] = ` [ Point { "x": 600, @@ -21,7 +21,7 @@ exports[`ActiveSelection adding to canvas sets coords 1`] = ` ] `; -exports[`ActiveSelection adding to canvas sets coords 2`] = ` +exports[`ActiveSelection sets coords after attaching to canvas 2`] = ` [ Point { "x": 100, From 1955e436a0bab353fd7be75e3d328bee060f8677 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sun, 10 Sep 2023 07:52:53 +0530 Subject: [PATCH 6/8] apply review comments --- src/shapes/ActiveSelection.spec.ts | 4 +- src/shapes/ActiveSelection.ts | 13 ++++++ .../ActiveSelection.spec.ts.snap | 44 +++++++++---------- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index e5469c4b3ae..5f527edd83e 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -69,7 +69,7 @@ describe('ActiveSelection', () => { ]), viewportTransform: [2, 0, 0, 0.5, 400, 150], }); - expect(canvas.getActiveSelection().getCoords()).toMatchSnapshot(); - expect(canvas.getActiveSelection().getCoords(true)).toMatchSnapshot(); + expect(canvas.getActiveSelection().lineCoords).toMatchSnapshot(); + expect(canvas.getActiveSelection().aCoords).toMatchSnapshot(); }); }); diff --git a/src/shapes/ActiveSelection.ts b/src/shapes/ActiveSelection.ts index be48541f531..71bc0eadabe 100644 --- a/src/shapes/ActiveSelection.ts +++ b/src/shapes/ActiveSelection.ts @@ -10,6 +10,19 @@ export interface ActiveSelectionOptions extends GroupProps { multiSelectionStacking: MultiSelectionStacking; } +/** + * Used by Canvas to manage selection. + * Canvas accepts an `activeSelection` option allowing overriding and customization. + * + * @example + * class MyActiveSelection extends ActiveSelection { + * ... + * } + * + * const canvas = new Canvas(el, { + * activeSelection: new MyActiveSelection() + * }) + */ export class ActiveSelection extends Group { declare _objects: FabricObject[]; diff --git a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap index 101e1d18ee8..de2c2463fe2 100644 --- a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap +++ b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap @@ -1,43 +1,43 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`ActiveSelection sets coords after attaching to canvas 1`] = ` -[ - Point { +{ + "bl": Point { "x": 600, - "y": 200, - }, - Point { - "x": 802, - "y": 200, + "y": 250.5, }, - Point { + "br": Point { "x": 802, "y": 250.5, }, - Point { + "tl": Point { "x": 600, - "y": 250.5, + "y": 200, + }, + "tr": Point { + "x": 802, + "y": 200, }, -] +} `; exports[`ActiveSelection sets coords after attaching to canvas 2`] = ` -[ - Point { +{ + "bl": Point { "x": 100, - "y": 100, - }, - Point { - "x": 201, - "y": 100, + "y": 201, }, - Point { + "br": Point { "x": 201, "y": 201, }, - Point { + "tl": Point { "x": 100, - "y": 201, + "y": 100, + }, + "tr": Point { + "x": 201, + "y": 100, }, -] +} `; From 2bc75a2d5a78b69b7c616708a4d580fb28258381 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sun, 10 Sep 2023 08:02:46 +0530 Subject: [PATCH 7/8] test2 Update ActiveSelection.spec.ts --- src/shapes/ActiveSelection.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index 5f527edd83e..cb4a0cb806b 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -57,6 +57,20 @@ describe('ActiveSelection', () => { expect(selection.item(0).getCenterPoint()).toEqual({ x: 50, y: 50 }); }); + // remove todo once #9152 is merged + it.todo('should not set coords in the constructor', () => { + const spy = jest.spyOn(ActiveSelection.prototype, 'setCoords'); + new ActiveSelection([ + new FabricObject({ + left: 100, + top: 100, + width: 100, + height: 100, + }), + ]); + expect(spy).not.toHaveBeenCalled(); + }); + it('sets coords after attaching to canvas', () => { const canvas = new Canvas(null, { activeSelection: new ActiveSelection([ From dca03c8350c3cd856e8a27de704a1c3564457eb5 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sun, 10 Sep 2023 12:17:51 +0530 Subject: [PATCH 8/8] skip Update ActiveSelection.spec.ts --- src/shapes/ActiveSelection.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index cb4a0cb806b..c7f6e393404 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -57,8 +57,8 @@ describe('ActiveSelection', () => { expect(selection.item(0).getCenterPoint()).toEqual({ x: 50, y: 50 }); }); - // remove todo once #9152 is merged - it.todo('should not set coords in the constructor', () => { + // remove skip once #9152 is merged + it.skip('should not set coords in the constructor', () => { const spy = jest.spyOn(ActiveSelection.prototype, 'setCoords'); new ActiveSelection([ new FabricObject({