diff --git a/CHANGELOG.md b/CHANGELOG.md index 52d43f42d4b..420ea40f9aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +- fix(Canvas): `setActiveObject` should update `canvas#_activeSelection` [#9336](https://github.com/fabricjs/fabric.js/pull/9336) + ## [next] - fix(Canvas): invalidate `_objectsToRender` on stack change [#9387](https://github.com/fabricjs/fabric.js/pull/9387) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 2eb88f853f4..fda896786af 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -293,7 +293,7 @@ export class SelectableCanvas protected declare _isCurrentlyDrawing: boolean; declare freeDrawingBrush?: BaseBrush; declare _activeObject?: FabricObject; - protected readonly _activeSelection: ActiveSelection; + protected _activeSelection: ActiveSelection; constructor( el?: string | HTMLCanvasElement, @@ -1086,6 +1086,12 @@ export class SelectableCanvas } this._activeObject = object; + if (object instanceof ActiveSelection && this._activeSelection !== object) { + this._activeSelection = object; + object.set('canvas', this); + object.setCoords(); + } + return true; } @@ -1161,7 +1167,7 @@ export class SelectableCanvas */ destroy() { // dispose of active selection - const activeSelection = this._activeSelection!; + const activeSelection = this._activeSelection; activeSelection.removeAll(); // @ts-expect-error disposing this._activeSelection = undefined; diff --git a/src/shapes/ActiveSelection.spec.ts b/src/shapes/ActiveSelection.spec.ts index bba71f69220..82088805a31 100644 --- a/src/shapes/ActiveSelection.spec.ts +++ b/src/shapes/ActiveSelection.spec.ts @@ -1,5 +1,6 @@ import { Canvas } from '../canvas/Canvas'; import { ActiveSelection } from './ActiveSelection'; +import { Group } from './Group'; import { FabricObject } from './Object/FabricObject'; describe('ActiveSelection', () => { @@ -86,4 +87,37 @@ describe('ActiveSelection', () => { expect(canvas.getActiveSelection().lineCoords).toMatchSnapshot(); expect(canvas.getActiveSelection().aCoords).toMatchSnapshot(); }); + + it('`setActiveObject` should update the active selection ref on canvas if it changed', () => { + const canvas = new Canvas(null); + const obj1 = new FabricObject(); + const obj2 = new FabricObject(); + canvas.add(obj1, obj2); + const activeSelection = new ActiveSelection([obj1, obj2]); + const spy = jest.spyOn(activeSelection, 'setCoords'); + canvas.setActiveObject(activeSelection); + expect(canvas.getActiveSelection()).toBe(activeSelection); + expect(canvas.getActiveObjects()).toEqual([obj1, obj2]); + expect(spy).toHaveBeenCalled(); + expect(activeSelection.canvas).toBe(canvas); + + spy.mockClear(); + canvas.setActiveObject(activeSelection); + expect(spy).not.toHaveBeenCalled(); + }); + + it('transferring an object between active selections keeps its owning group', () => { + const object = new FabricObject(); + const group = new Group([object]); + const activeSelection1 = new ActiveSelection([object]); + const activeSelection2 = new ActiveSelection(); + expect(object.group).toBe(activeSelection1); + expect(object.getParent(true)).toBe(group); + activeSelection2.add(object); + expect(object.group).toBe(activeSelection2); + expect(object.getParent(true)).toBe(group); + activeSelection2.removeAll(); + expect(object.group).toBe(group); + expect(object.getParent(true)).toBe(group); + }); }); diff --git a/src/shapes/ActiveSelection.ts b/src/shapes/ActiveSelection.ts index 71bc0eadabe..cd5adfde41a 100644 --- a/src/shapes/ActiveSelection.ts +++ b/src/shapes/ActiveSelection.ts @@ -86,7 +86,9 @@ export class ActiveSelection extends Group { // save ref to group for later in order to return to it const parent = object.group; parent._exitGroup(object); - object.__owningGroup = parent; + // make sure we are setting the correct owning group + // in case `object` is transferred between active selections + !(parent instanceof ActiveSelection) && (object.__owningGroup = parent); } this._enterGroup(object, removeParentTransform); return true;