From 4979eecce7b8d2a943611766740aa4650f33ac2f Mon Sep 17 00:00:00 2001 From: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Tue, 9 Jan 2024 11:48:53 +0200 Subject: [PATCH] BRAEKING BETA revert(): rm active selection ref (#9561) **BRAEKING beta** --------- Co-authored-by: github-actions[bot] Co-authored-by: Andrea Bogazzi --- .../templates/next/components/Canvas.tsx | 2 + CHANGELOG.md | 1 + src/ClassRegistry.ts | 2 +- src/canvas/Canvas.ts | 54 ++++++++------ src/canvas/CanvasOptions.ts | 5 +- src/canvas/SelectableCanvas.ts | 74 ++++++------------- src/canvas/StaticCanvas.ts | 2 +- src/filters/Composed.ts | 4 +- src/shapes/ActiveSelection.spec.ts | 22 +----- src/shapes/ActiveSelection.ts | 21 ++++-- src/shapes/Group.ts | 9 ++- src/shapes/Object/Object.ts | 2 +- .../ActiveSelection.spec.ts.snap | 22 ------ src/util/misc/objectEnlive.ts | 27 ++++--- src/util/typeAssertions.spec.ts | 16 ++++ src/util/typeAssertions.ts | 6 ++ test/unit/canvas.js | 43 ++++++----- test/unit/canvas_dispose.js | 13 ++-- test/unit/canvas_events.js | 2 +- 19 files changed, 157 insertions(+), 170 deletions(-) delete mode 100644 src/shapes/__snapshots__/ActiveSelection.spec.ts.snap diff --git a/.codesandbox/templates/next/components/Canvas.tsx b/.codesandbox/templates/next/components/Canvas.tsx index 181c8c2bf27..3f2d6400430 100644 --- a/.codesandbox/templates/next/components/Canvas.tsx +++ b/.codesandbox/templates/next/components/Canvas.tsx @@ -28,6 +28,8 @@ export const Canvas = React.forwardRef< ref.current = canvas; } + // it is crucial `onLoad` is a dependency of this effect + // to ensure the canvas is disposed and re-created if it changes onLoad?.(canvas); return () => { diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fb9ccedd26..f4fa1fa01ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- refactor(): rm active selection ref [#9561](https://github.com/fabricjs/fabric.js/pull/9561) - feat(Next.js sandbox): simpler canvas hook [#9577](https://github.com/fabricjs/fabric.js/pull/9577) - fix(): fix modify polygon points with zero sized polygons ( particular case of axis oriented lines ) [#9575](https://github.com/fabricjs/fabric.js/pull/9575) - fix(Polyline, Polygon): Fix wrong pathOffset for polyline with the normal bounding box calculation. [#9460](https://github.com/fabricjs/fabric.js/pull/9460) diff --git a/src/ClassRegistry.ts b/src/ClassRegistry.ts index 67835cd76d2..8b673c40686 100644 --- a/src/ClassRegistry.ts +++ b/src/ClassRegistry.ts @@ -24,7 +24,7 @@ export class ClassRegistry { this[SVG] = new Map(); } - getClass(classType: string): any { + getClass(classType: string): T { const constructor = this[JSON].get(classType); if (!constructor) { throw new FabricError(`No class registered for ${classType}`); diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index 06d05af2880..81eb115d4ac 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -1,3 +1,4 @@ +import { classRegistry } from '../ClassRegistry'; import { NONE } from '../constants'; import type { CanvasEvents, @@ -8,12 +9,14 @@ import type { Transform, } from '../EventTypeDefs'; import { Point } from '../Point'; +import type { ActiveSelection } from '../shapes/ActiveSelection'; import type { Group } from '../shapes/Group'; import type { IText } from '../shapes/IText/IText'; import type { FabricObject } from '../shapes/Object/FabricObject'; import { isTouchEvent, stopEvent } from '../util/dom_event'; import { getDocumentFromElement, getWindowFromElement } from '../util/dom_misc'; import { sendPointToPlane } from '../util/misc/planeChange'; +import { isActiveSelection } from '../util/typeAssertions'; import type { CanvasOptions, TCanvasOptions } from './CanvasOptions'; import { SelectableCanvas } from './SelectableCanvas'; import { TextEditingManager } from './TextEditingManager'; @@ -1387,10 +1390,9 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { return; } let hoverCursor = target.hoverCursor || this.hoverCursor; - const activeSelection = - this._activeObject === this._activeSelection - ? this._activeObject - : null, + const activeSelection = isActiveSelection(this._activeObject) + ? this._activeObject + : null, // only show proper corner when group selection is not active corner = (!activeSelection || target.group !== activeSelection) && @@ -1431,8 +1433,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { */ protected handleMultiSelection(e: TPointerEvent, target?: FabricObject) { const activeObject = this._activeObject; - const activeSelection = this._activeSelection; - const isAS = activeObject === activeSelection; + const isAS = isActiveSelection(activeObject); if ( // check if an active object exists on canvas and if the user is pressing the `selectionKey` while canvas supports multi selection. !!activeObject && @@ -1455,9 +1456,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { !activeObject.getActiveControl() ) { if (isAS) { - const prevActiveObjects = - activeSelection.getObjects() as FabricObject[]; - if (target === activeSelection) { + const prevActiveObjects = activeObject.getObjects(); + if (target === activeObject) { const pointer = this.getViewportPoint(e); target = // first search active objects for a target to remove @@ -1470,19 +1470,21 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { return false; } } - if (target.group === activeSelection) { + if (target.group === activeObject) { // `target` is part of active selection => remove it - activeSelection.remove(target); + activeObject.remove(target); this._hoveredTarget = target; this._hoveredTargets = [...this.targets]; - if (activeSelection.size() === 1) { + // if after removing an object we are left with one only... + if (activeObject.size() === 1) { // activate last remaining object - this._setActiveObject(activeSelection.item(0) as FabricObject, e); + // deselecting the active selection will remove the remaining object from it + this._setActiveObject(activeObject.item(0), e); } } else { - // `target` isn't part of active selection => add it - activeSelection.multiSelectAdd(target); - this._hoveredTarget = activeSelection; + // `target` isn't part of active selection => add it + activeObject.multiSelectAdd(target); + this._hoveredTarget = activeObject; this._hoveredTargets = [...this.targets]; } this._fireSelectionEvents(prevActiveObjects, e); @@ -1490,12 +1492,21 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { (activeObject as IText).exitEditing && (activeObject as IText).exitEditing(); // add the active object and the target to the active selection and set it as the active object - activeSelection.multiSelectAdd(activeObject, target); - this._hoveredTarget = activeSelection; + const klass = + classRegistry.getClass('ActiveSelection'); + const newActiveSelection = new klass([], { + /** + * it is crucial to pass the canvas ref before calling {@link ActiveSelection#multiSelectAdd} + * since it uses {@link FabricObject#isInFrontOf} which relies on the canvas ref + */ + canvas: this, + }); + newActiveSelection.multiSelectAdd(activeObject, target); + this._hoveredTarget = newActiveSelection; // ISSUE 4115: should we consider subTargets here? // this._hoveredTargets = []; // this._hoveredTargets = this.targets.concat(); - this._setActiveObject(activeSelection, e); + this._setActiveObject(newActiveSelection, e); this._fireSelectionEvents([activeObject], e); } return true; @@ -1549,8 +1560,9 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { this.setActiveObject(objects[0], e); } else if (objects.length > 1) { // add to active selection and make it the active object - this._activeSelection.add(...objects); - this.setActiveObject(this._activeSelection, e); + const klass = + classRegistry.getClass('ActiveSelection'); + this.setActiveObject(new klass(objects, { canvas: this }), e); } // cleanup diff --git a/src/canvas/CanvasOptions.ts b/src/canvas/CanvasOptions.ts index 7cb12c3256e..471416d17cf 100644 --- a/src/canvas/CanvasOptions.ts +++ b/src/canvas/CanvasOptions.ts @@ -1,5 +1,4 @@ import type { ModifierKey, TOptionalModifierKey } from '../EventTypeDefs'; -import type { ActiveSelection } from '../shapes/ActiveSelection'; import type { TOptions } from '../typedefs'; import type { StaticCanvasOptions } from './StaticCanvasOptions'; @@ -260,9 +259,7 @@ export interface CanvasOptions preserveObjectStacking: boolean; } -export type TCanvasOptions = TOptions< - CanvasOptions & { activeSelection: ActiveSelection } ->; +export type TCanvasOptions = TOptions; export const canvasDefaults: TOptions = { uniformScaling: true, diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 94af4847b75..396595c4f4e 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -11,7 +11,6 @@ import type { } from '../EventTypeDefs'; import { addTransformToObject, - resetObjectTransform, saveObjectTransform, } from '../util/misc/objectTransforms'; import type { TCanvasSizeOptions } from './StaticCanvas'; @@ -31,13 +30,13 @@ import type { IText } from '../shapes/IText/IText'; import type { BaseBrush } from '../brushes/BaseBrush'; import { pick } from '../util/misc/pick'; import { sendPointToPlane } from '../util/misc/planeChange'; -import { ActiveSelection } from '../shapes/ActiveSelection'; import { cos, createCanvasElement, sin } from '../util'; import { CanvasDOMManager } from './DOMManagers/CanvasDOMManager'; import { BOTTOM, CENTER, LEFT, RIGHT, TOP } from '../constants'; -import type { CanvasOptions, TCanvasOptions } from './CanvasOptions'; +import type { CanvasOptions } from './CanvasOptions'; import { canvasDefaults } from './CanvasOptions'; import { Intersection } from '../Intersection'; +import { isActiveSelection } from '../util/typeAssertions'; /** * Canvas class @@ -294,17 +293,6 @@ export class SelectableCanvas protected declare _isCurrentlyDrawing: boolean; declare freeDrawingBrush?: BaseBrush; declare _activeObject?: FabricObject; - protected _activeSelection: ActiveSelection; - - constructor( - el?: string | HTMLCanvasElement, - { activeSelection = new ActiveSelection(), ...options }: TCanvasOptions = {} - ) { - super(el, options); - this._activeSelection = activeSelection; - this._activeSelection.set('canvas', this); - this._activeSelection.setCoords(); - } protected initElements(el?: string | HTMLCanvasElement) { this.elements = new CanvasDOMManager(el, { @@ -1029,27 +1017,17 @@ export class SelectableCanvas return this._activeObject; } - /** - * Returns instance's active selection - */ - getActiveSelection() { - return this._activeSelection; - } - /** * Returns an array with the current selected objects * @return {FabricObject[]} active objects array */ getActiveObjects(): FabricObject[] { const active = this._activeObject; - if (active) { - if (active === this._activeSelection) { - return [...(active as ActiveSelection)._objects]; - } else { - return [active]; - } - } - return []; + return isActiveSelection(active) + ? active.getObjects() + : active + ? [active] + : []; } /** @@ -1134,9 +1112,11 @@ export class SelectableCanvas * @return {Boolean} true if the object has been selected */ _setActiveObject(object: FabricObject, e?: TPointerEvent) { - if (this._activeObject === object) { + const prevActiveObject = this._activeObject; + if (prevActiveObject === object) { return false; } + // after calling this._discardActiveObject, this,_activeObject could be undefined if (!this._discardActiveObject(e, object) && this._activeObject) { // refused to deselect return false; @@ -1144,10 +1124,10 @@ export class SelectableCanvas if (object.onSelect({ e })) { return false; } + this._activeObject = object; - if (object instanceof ActiveSelection && this._activeSelection !== object) { - this._activeSelection = object; + if (isActiveSelection(object) && prevActiveObject !== object) { object.set('canvas', this); object.setCoords(); } @@ -1173,11 +1153,6 @@ export class SelectableCanvas if (obj.onDeselect({ e, object })) { return false; } - // clear active selection - if (obj === this._activeSelection) { - this._activeSelection.removeAll(); - resetObjectTransform(this._activeSelection); - } if (this._currentTransform && this._currentTransform.target === obj) { // @ts-expect-error this method exists in the subclass - should be moved or declared as abstract this.endCurrentTransform(e); @@ -1227,11 +1202,13 @@ export class SelectableCanvas */ destroy() { // dispose of active selection - const activeSelection = this._activeSelection; - activeSelection.removeAll(); - // @ts-expect-error disposing - this._activeSelection = undefined; - activeSelection.dispose(); + const activeObject = this._activeObject; + if (isActiveSelection(activeObject)) { + activeObject.removeAll(); + activeObject.dispose(); + } + + delete this._activeObject; super.destroy(); @@ -1271,7 +1248,7 @@ export class SelectableCanvas /** * @private */ - _toObject( + protected _toObject( instance: FabricObject, methodName: 'toObject' | 'toDatalessObject', propertiesToInclude: string[] @@ -1293,14 +1270,11 @@ export class SelectableCanvas * @param {FabricObject} [instance] the object to transform (gets mutated) * @returns the original values of instance which were changed */ - _realizeGroupTransformOnObject( + private _realizeGroupTransformOnObject( instance: FabricObject ): Partial { - if ( - instance.group && - instance.group === this._activeSelection && - this._activeObject === instance.group - ) { + const { group } = instance; + if (group && isActiveSelection(group) && this._activeObject === group) { const layoutProps = [ 'angle', 'flipX', @@ -1313,7 +1287,7 @@ export class SelectableCanvas TOP, ] as (keyof typeof instance)[]; const originalValues = pick(instance, layoutProps); - addTransformToObject(instance, this._activeObject.calcOwnMatrix()); + addTransformToObject(instance, group.calcOwnMatrix()); return originalValues; } else { return {}; diff --git a/src/canvas/StaticCanvas.ts b/src/canvas/StaticCanvas.ts index ae9d295c109..6fa83def8cc 100644 --- a/src/canvas/StaticCanvas.ts +++ b/src/canvas/StaticCanvas.ts @@ -859,7 +859,7 @@ export class StaticCanvas< /** * @private */ - _toObject( + protected _toObject( instance: FabricObject, methodName: TValidToObjectMethod, propertiesToInclude?: string[] diff --git a/src/filters/Composed.ts b/src/filters/Composed.ts index 54243eb1ef0..198a86a08c5 100644 --- a/src/filters/Composed.ts +++ b/src/filters/Composed.ts @@ -67,7 +67,9 @@ export class Composed extends BaseFilter { ) { return Promise.all( ((object.subFilters || []) as BaseFilter[]).map((filter) => - classRegistry.getClass(filter.type).fromObject(filter, options) + classRegistry + .getClass(filter.type) + .fromObject(filter, options) ) ).then( (enlivedFilters) => new this({ subFilters: enlivedFilters }) as BaseFilter diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index 185d0c2b400..9ee419c650a 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -81,21 +81,6 @@ describe('ActiveSelection', () => { expect(spy).not.toHaveBeenCalled(); }); - it('sets coords after attaching to canvas', () => { - const canvas = new Canvas(undefined, { - activeSelection: new ActiveSelection([ - new FabricObject({ - left: 100, - top: 100, - width: 100, - height: 100, - }), - ]), - viewportTransform: [2, 0, 0, 0.5, 400, 150], - }); - expect(canvas.getActiveSelection().aCoords).toMatchSnapshot(); - }); - it('`setActiveObject` should update the active selection ref on canvas if it changed', () => { const canvas = new Canvas(); const obj1 = new FabricObject(); @@ -104,7 +89,7 @@ describe('ActiveSelection', () => { const activeSelection = new ActiveSelection([obj1, obj2]); const spy = jest.spyOn(activeSelection, 'setCoords'); canvas.setActiveObject(activeSelection); - expect(canvas.getActiveSelection()).toBe(activeSelection); + expect(canvas.getActiveObject()).toBe(activeSelection); expect(canvas.getActiveObjects()).toEqual([obj1, obj2]); expect(spy).toHaveBeenCalled(); expect(activeSelection.canvas).toBe(canvas); @@ -117,8 +102,7 @@ describe('ActiveSelection', () => { test('adding and removing an object belonging to a group', () => { const object = new FabricObject(); const group = new Group([object]); - const canvas = new Canvas(); - const activeSelection = canvas.getActiveSelection(); + const activeSelection = new ActiveSelection(); const eventsSpy = jest.spyOn(object, 'fire'); const removeSpy = jest.spyOn(group, 'remove'); @@ -132,7 +116,6 @@ describe('ActiveSelection', () => { activeSelection.add(object); expect(object.group).toBe(activeSelection); expect(object.parent).toBe(group); - expect(object.canvas).toBe(canvas); expect(removeSpy).not.toBeCalled(); expect(exitSpy).toBeCalledWith(object); expect(enterSpy).toBeCalledWith(object, true); @@ -146,7 +129,6 @@ describe('ActiveSelection', () => { }); expect(object.group).toBe(group); expect(object.parent).toBe(group); - expect(object.canvas).toBeUndefined(); }); test('transferring an object between active selections', () => { diff --git a/src/shapes/ActiveSelection.ts b/src/shapes/ActiveSelection.ts index d52050fadbf..cb8d9822841 100644 --- a/src/shapes/ActiveSelection.ts +++ b/src/shapes/ActiveSelection.ts @@ -16,18 +16,26 @@ export interface ActiveSelectionOptions extends GroupProps { /** * 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() - * }) + * // override the default `ActiveSelection` class + * classRegistry.setClass(MyActiveSelection) */ export class ActiveSelection extends Group { + static type = 'ActiveSelection'; + + static ownDefaults: Record = { + multiSelectionStacking: 'canvas-stacking', + }; + + static getDefaults() { + return { ...super.getDefaults(), ...this.ownDefaults }; + } + /** * controls how selected objects are added during a multiselection event * - `canvas-stacking` adds the selected object to the active selection while respecting canvas object stacking order @@ -35,10 +43,7 @@ export class ActiveSelection extends Group { * meaning that the stack is ordered by the order in which objects were selected * @default `canvas-stacking` */ - // TODO FIX THIS WITH THE DEFAULTS LOGIC - multiSelectionStacking: MultiSelectionStacking = 'canvas-stacking'; - - static type = 'ActiveSelection'; + declare multiSelectionStacking: MultiSelectionStacking; /** * @private diff --git a/src/shapes/Group.ts b/src/shapes/Group.ts index 1ada7c755b8..56c92104eff 100644 --- a/src/shapes/Group.ts +++ b/src/shapes/Group.ts @@ -28,6 +28,7 @@ import { LAYOUT_TYPE_REMOVED, } from '../LayoutManager/constants'; import type { SerializedLayoutManager } from '../LayoutManager/LayoutManager'; +import type { FitContentLayout } from '../LayoutManager'; /** * This class handles the specific case of creating a group using {@link Group#fromObject} and is not meant to be used in any other case. @@ -661,8 +662,12 @@ export class Group layoutManager: new NoopLayoutManager(), }); if (layoutManager) { - const layoutClass = classRegistry.getClass(layoutManager.type); - const strategyClass = classRegistry.getClass(layoutManager.strategy); + const layoutClass = classRegistry.getClass( + layoutManager.type + ); + const strategyClass = classRegistry.getClass( + layoutManager.strategy + ); group.layoutManager = new layoutClass(new strategyClass()); } else { group.layoutManager = new LayoutManager(); diff --git a/src/shapes/Object/Object.ts b/src/shapes/Object/Object.ts index d6ca9d23815..8e254469b10 100644 --- a/src/shapes/Object/Object.ts +++ b/src/shapes/Object/Object.ts @@ -1342,7 +1342,7 @@ export class FabricObject< cloneAsImage(options: any): FabricImage { const canvasEl = this.toCanvasElement(options); // TODO: how to import Image w/o an import cycle? - const ImageClass = classRegistry.getClass('image'); + const ImageClass = classRegistry.getClass('image'); return new ImageClass(canvasEl); } diff --git a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap b/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap deleted file mode 100644 index e3bd6395ba5..00000000000 --- a/src/shapes/__snapshots__/ActiveSelection.spec.ts.snap +++ /dev/null @@ -1,22 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ActiveSelection sets coords after attaching to canvas 1`] = ` -{ - "bl": Point { - "x": 100, - "y": 201, - }, - "br": Point { - "x": 201, - "y": 201, - }, - "tl": Point { - "x": 100, - "y": 100, - }, - "tr": Point { - "x": 201, - "y": 100, - }, -} -`; diff --git a/src/util/misc/objectEnlive.ts b/src/util/misc/objectEnlive.ts index 16b90498933..e160f85baeb 100644 --- a/src/util/misc/objectEnlive.ts +++ b/src/util/misc/objectEnlive.ts @@ -1,12 +1,18 @@ import { noop } from '../../constants'; import type { Pattern } from '../../Pattern'; import type { FabricObject } from '../../shapes/Object/FabricObject'; -import type { Abortable, TCrossOrigin, TFiller } from '../../typedefs'; +import type { + Abortable, + Constructor, + TCrossOrigin, + TFiller, +} from '../../typedefs'; import { createImage } from './dom'; import { classRegistry } from '../../ClassRegistry'; import type { BaseFilter } from '../../filters/BaseFilter'; import type { FabricObject as BaseFabricObject } from '../../shapes/Object/Object'; import { FabricError, SignalAbortedError } from '../internals/console'; +import type { Gradient } from '../../gradient'; export type LoadImageOptions = Abortable & { /** @@ -88,13 +94,14 @@ export const enlivenObjects = < Promise.all( objects.map((obj) => classRegistry - .getClass(obj.type) - .fromObject(obj, { - signal, - reviver, - }) - .then((fabricInstance: T) => { - reviver(obj, fabricInstance); + .getClass< + Constructor & { + fromObject(options: any, context: Abortable): Promise; + } + >(obj.type) + .fromObject(obj, { signal }) + .then((fabricInstance) => { + reviver(obj, fabricInstance); instances.push(fabricInstance); return fabricInstance; }) @@ -137,7 +144,7 @@ export const enlivenObjectEnlivables = < } // gradient if (value.colorStops) { - return new (classRegistry.getClass('gradient'))(value); + return new (classRegistry.getClass('gradient'))(value); } // clipPath if (value.type) { @@ -151,7 +158,7 @@ export const enlivenObjectEnlivables = < // pattern if (value.source) { return classRegistry - .getClass('pattern') + .getClass('pattern') .fromObject(value, { signal }) .then((pattern: Pattern) => { instances.push(pattern); diff --git a/src/util/typeAssertions.spec.ts b/src/util/typeAssertions.spec.ts index 7e916ff5f20..c3a9721283c 100644 --- a/src/util/typeAssertions.spec.ts +++ b/src/util/typeAssertions.spec.ts @@ -4,6 +4,7 @@ import { isPattern, isPath, isFiller, + isActiveSelection, } from './typeAssertions'; import { FabricText } from '../shapes/Text/Text'; import { IText } from '../shapes/IText/IText'; @@ -12,6 +13,9 @@ import { Path } from '../shapes/Path'; import { Pattern } from '../Pattern/Pattern'; import { Gradient } from '../gradient/Gradient'; import { Shadow } from '../Shadow'; +import { ActiveSelection } from '../shapes/ActiveSelection'; +import { Canvas } from '../canvas/Canvas'; +import { Group } from '../shapes/Group'; describe('typeAssertions', () => { describe('isTextObject', () => { @@ -106,4 +110,16 @@ describe('typeAssertions', () => { expect(isSerializableFiller(gradient)).toBe(true); }); }); + describe('isActiveSelection', () => { + test('can detect activeSelection', () => { + const as = new ActiveSelection([], { + canvas: new Canvas(), + }); + expect(isActiveSelection(as)).toBe(true); + }); + test('can safeguard against a group', () => { + const group = new Group([]); + expect(isActiveSelection(group)).toBe(false); + }); + }); }); diff --git a/src/util/typeAssertions.ts b/src/util/typeAssertions.ts index ac7751a7cb1..765da5a8ab9 100644 --- a/src/util/typeAssertions.ts +++ b/src/util/typeAssertions.ts @@ -3,6 +3,7 @@ import type { TFiller } from '../typedefs'; import type { FabricText } from '../shapes/Text/Text'; import type { Pattern } from '../Pattern'; import type { Path } from '../shapes/Path'; +import type { ActiveSelection } from '../shapes/ActiveSelection'; export const isFiller = ( filler: TFiller | string | null @@ -41,3 +42,8 @@ export const isPath = (fabricObject?: FabricObject): fabricObject is Path => { typeof (fabricObject as Path)._renderPathCommands === 'function' ); }; + +export const isActiveSelection = ( + fabricObject?: FabricObject +): fabricObject is ActiveSelection => + !!fabricObject && Object.hasOwn(fabricObject, 'multiSelectionStacking'); diff --git a/test/unit/canvas.js b/test/unit/canvas.js index 45f56c82617..a5737d76b52 100644 --- a/test/unit/canvas.js +++ b/test/unit/canvas.js @@ -100,6 +100,7 @@ }, afterEach: function () { fabric.config.restoreDefaults(); + fabric.classRegistry.setClass(fabric.ActiveSelection); return canvas.dispose(); } }); @@ -315,7 +316,7 @@ var rect1 = new fabric.Rect(); var rect2 = new fabric.Rect(); canvas.add(rect1, rect2); - var activeSelection = canvas.getActiveSelection(); + var activeSelection = new fabric.ActiveSelection(); activeSelection.add(rect1, rect2); canvas.setActiveObject(activeSelection); canvas.discardActiveObject(); @@ -341,18 +342,21 @@ }); function initActiveSelection(canvas, activeObject, target, multiSelectionStacking) { - const activeSelection = canvas.getActiveSelection(); - activeSelection.multiSelectionStacking = multiSelectionStacking; + fabric.classRegistry.setClass(class TextActiveSelection extends fabric.ActiveSelection { + static getDefaults() { + return {...super.getDefaults(),multiSelectionStacking} + } + }); canvas.setActiveObject(activeObject); canvas.handleMultiSelection({ clientX: 0, clientY: 0, [canvas.selectionKey]: true }, target); } function updateActiveSelection(canvas, existing, target, multiSelectionStacking) { - const activeSelection = canvas.getActiveSelection(); + const activeSelection = new fabric.ActiveSelection([], {canvas}); activeSelection.multiSelectionStacking = multiSelectionStacking; activeSelection.add(...existing); canvas.setActiveObject(activeSelection); - canvas.handleMultiSelection({ clientX: 1, clientY: 1, [canvas.selectionKey]: true, target: canvas.upperCanvasEl }, target); + canvas.handleMultiSelection({ clientX: 1, clientY: 1, [canvas.selectionKey]: true, target: canvas.upperCanvasEl }, target || activeSelection); } QUnit.test('create active selection fires selection:created', function(assert) { @@ -504,7 +508,7 @@ let isFired = false; rect3.on('deselected', () => { isFired = true; }); canvas.add(rect1, rect2, rect3); - updateActiveSelection(canvas, [rect1, rect2, rect3], canvas.getActiveSelection(), 'selection-order'); + updateActiveSelection(canvas, [rect1, rect2, rect3], null, 'selection-order'); assert.deepEqual(canvas.getActiveObjects(), [rect1, rect2], 'rect3 was deselected'); assert.ok(isFired, 'fired deselected'); }); @@ -514,9 +518,9 @@ const rect2 = new fabric.Rect({ left: -10, width: 5, height: 5 }); const rect3 = new fabric.Rect({ top: 10, width: 10, height: 10 }); canvas.add(rect1, rect2, rect3); - updateActiveSelection(canvas, [rect1, rect2, rect3], canvas.getActiveSelection(), 'selection-order'); + updateActiveSelection(canvas, [rect1, rect2, rect3], null, 'selection-order'); assert.deepEqual(canvas.getActiveObjects(), [rect1, rect2, rect3], 'nothing happened'); - assert.ok(canvas.getActiveSelection() === canvas.getActiveObject(), 'still selected'); + assert.ok(canvas.getActiveObject() === canvas.getActiveObject(), 'still selected'); }); QUnit.test('multiselection: selecting a target behind active selection', assert => { @@ -525,11 +529,11 @@ const rect3 = new fabric.Rect({ top: 10, width: 10, height: 10 }); canvas.add(rect1, rect2, rect3); initActiveSelection(canvas, rect1, rect3); - assert.ok(canvas.getActiveSelection() === canvas.getActiveObject(), 'selected'); + assert.ok(canvas.getActiveObject() === canvas.getActiveObject(), 'selected'); assert.deepEqual(canvas.getActiveObjects(), [rect1, rect3], 'created'); canvas.__onMouseDown({ clientX: 7, clientY: 7, [canvas.selectionKey]: true }); assert.deepEqual(canvas.getActiveObjects(), [rect1, rect2, rect3], 'added from behind active selection'); - assert.ok(canvas.getActiveSelection() === canvas.getActiveObject(), 'still selected'); + assert.ok(canvas.getActiveObject() === canvas.getActiveObject(), 'still selected'); }); QUnit.test('setActiveObject fires deselected', function(assert) { @@ -711,7 +715,7 @@ var rect1 = new fabric.Rect(); var rect2 = new fabric.Rect(); var rect3 = new fabric.Rect(); - var activeSelection = canvas.getActiveSelection(); + var activeSelection = new fabric.ActiveSelection(); activeSelection.add(rect1, rect2); canvas.setActiveObject(activeSelection); rect1.on('deselected', function( ) { @@ -1095,7 +1099,7 @@ canvas.add(rect1); canvas.add(rect2); canvas.add(rect3); - const group = canvas.getActiveSelection(); + const group = new fabric.ActiveSelection(); group.subTargetCheck = true; group.add(rect1, rect2); group.cornerSize = 2; @@ -1139,7 +1143,7 @@ canvas.preserveObjectStacking = true; canvas.add(rect1); canvas.add(rect2); - const group = canvas.getActiveSelection(); + const group = new fabric.ActiveSelection(); group.add(rect1, rect2); canvas.setActiveObject(group); target = canvas.findTarget({ @@ -1218,7 +1222,7 @@ canvas.add(rect, circle); var json = JSON.stringify(canvas); - const activeSelection = canvas.getActiveSelection(); + const activeSelection = new fabric.ActiveSelection(); activeSelection.add(rect, circle); canvas.setActiveObject(activeSelection); var jsonWithActiveGroup = JSON.stringify(canvas); @@ -1760,11 +1764,6 @@ assert.equal(canvas.getActiveObject(), group); }); - QUnit.test('getActiveSelection', function(assert) { - assert.ok(canvas.getActiveSelection() === canvas._activeSelection, 'should equal'); - assert.ok(canvas.getActiveSelection() instanceof fabric.ActiveSelection, 'is active selection'); - }); - QUnit.test('item', function(assert) { assert.ok(typeof canvas.item === 'function'); @@ -1782,7 +1781,7 @@ }); QUnit.test('discardActiveObject on ActiveSelection', function(assert) { - var group = new fabric.ActiveSelection([makeRect(), makeRect()]); + var group = new fabric.ActiveSelection([makeRect(), makeRect()], {canvas}); canvas.setActiveObject(group); canvas.discardActiveObject(); assert.equal(canvas.getActiveObject(), null, 'removing active group sets it to null'); @@ -1878,7 +1877,7 @@ var circle = new fabric.Circle({ radius: 50, left: 50, top: 50 }); canvas.add(rect, circle); var svg = canvas.toSVG(); - const activeSelection = canvas.getActiveSelection(); + const activeSelection = new fabric.ActiveSelection(); activeSelection.add(rect, circle); canvas.setActiveObject(activeSelection); var svgWithActiveGroup = canvas.toSVG(); @@ -2272,7 +2271,7 @@ { start: 28, end: 33, message: 'stroke tolerance not affected by vpt', transparent: false }, { start: 33, end: 40, message: 'outside', transparent: true }, ]); - }); + }); }); QUnit.test('canvas getTopContext', function(assert) { diff --git a/test/unit/canvas_dispose.js b/test/unit/canvas_dispose.js index d94a7a36b2a..320a06111b7 100644 --- a/test/unit/canvas_dispose.js +++ b/test/unit/canvas_dispose.js @@ -47,7 +47,7 @@ function assertCanvasDisposing(klass) { assert.equal(el.height, 200, 'restored height'); }); - + QUnit.test('dispose: clear references async', async function (assert) { const canvas = new klass(null, { renderOnAddRemove: false }); assert.ok(typeof canvas.dispose === 'function'); @@ -248,7 +248,7 @@ function testCanvasDisposing() { wrapperEl = canvas.wrapperEl; lowerCanvasEl = canvas.lowerCanvasEl; upperCanvasEl = canvas.upperCanvasEl; - const activeSel = canvas.getActiveSelection(); + const activeSel = new fabric.ActiveSelection(); assert.equal(parentEl.childNodes.length, 1, 'parentEl has still 1 child only'); assert.equal(wrapperEl.childNodes.length, 2, 'wrapper should have 2 children'); assert.equal(wrapperEl.tagName, 'DIV', 'We wrapped canvas with DIV'); @@ -306,7 +306,8 @@ function testCanvasDisposing() { wrapperEl = canvas.wrapperEl; lowerCanvasEl = canvas.lowerCanvasEl; upperCanvasEl = canvas.upperCanvasEl; - const activeSel = canvas.getActiveSelection(); + const activeSel = new fabric.ActiveSelection(); + canvas.setActiveObject(activeSel) assert.equal(parentEl.childNodes.length, 1, 'parentEl has still 1 child only'); assert.equal(wrapperEl.childNodes.length, 2, 'wrapper should have 2 children'); assert.equal(wrapperEl.tagName, 'DIV', 'We wrapped canvas with DIV'); @@ -314,7 +315,7 @@ function testCanvasDisposing() { assert.equal(wrapperEl.childNodes[0], lowerCanvasEl, 'First child should be lowerCanvas'); assert.equal(wrapperEl.childNodes[1], upperCanvasEl, 'Second child should be upperCanvas'); assert.equal(canvas.elements._originalCanvasStyle, elStyle, 'saved original canvas style for disposal'); - assert.ok(activeSel instanceof fabric.ActiveSelection, 'active selection'); + assert.ok(canvas.getActiveObject() === activeSel, 'active selection'); assert.notEqual(el.style.cssText, canvas.elements._originalCanvasStyle, 'canvas el style has been changed'); if (!isNode()) { assert.equal(parentEl.childNodes[0], wrapperEl, 'wrapperEl is appended to rootNode'); @@ -322,7 +323,7 @@ function testCanvasDisposing() { //looks like i cannot use parentNode //equal(wrapperEl, lowerCanvasEl.parentNode, 'lowerCanvas is appended to wrapperEl'); //equal(wrapperEl, upperCanvasEl.parentNode, 'upperCanvas is appended to wrapperEl'); - //equal(parentEl, wrapperEl.parentNode, 'wrapperEl is appendend to rootNode'); + //equal(parentEl, wrapperEl.parentNode, 'wrapperEl is appended to rootNode'); assert.equal(parentEl.childNodes.length, 1, 'parent div should have 1 child'); assert.notEqual(parentEl.firstChild, canvas.getElement(), 'canvas should not be parent div firstChild'); assert.ok(typeof canvas.dispose === 'function'); @@ -334,7 +335,7 @@ function testCanvasDisposing() { await canvas.dispose(); assert.equal(fabric.runningAnimations.length, 0, 'dispose should clear running animations'); assert.equal(canvas.getObjects().length, 0, 'dispose should clear canvas'); - assert.equal(canvas.getActiveSelection(), undefined, 'dispose should dispose active selection'); + assert.equal(canvas.getActiveObject(), undefined, 'dispose should dispose active selection'); assert.equal(activeSel.size(), 0, 'dispose should dispose active selection'); assert.equal(parentEl.childNodes.length, 1, 'parent has always 1 child'); if (!isNode()) { diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index 030441be5a3..ada6c5c9c37 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -759,7 +759,7 @@ }); } canvas.loadFromJSON(SUB_TARGETS_JSON).then(function() { - var activeSelection = canvas.getActiveSelection(); + var activeSelection = new fabric.ActiveSelection(); activeSelection.add(...canvas.getObjects()); canvas.setActiveObject(activeSelection); setSubTargetCheckRecursive(activeSelection);