diff --git a/CHANGELOG.md b/CHANGELOG.md index cfc38981b1f..7fffed3858e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(): BREAKING set/discard active object return value, discard active object now return false if no discard happened [#8672](https://github.com/fabricjs/fabric.js/issues/8672) - fix(): selection logic to support nested multiselection [#8665](https://github.com/fabricjs/fabric.js/issues/8665) - fix(test): remove bad node config [#8694](https://github.com/fabricjs/fabric.js/issues/8694) - fix(): keep browser files as .js [#8690](https://github.com/fabricjs/fabric.js/issues/8690) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 3c07c113733..e142d7b6c3f 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -19,7 +19,7 @@ import { StaticCanvas, TCanvasSizeOptions } from './StaticCanvas'; import { isCollection, isFabricObjectCached } from '../util/types'; import { invertTransform, transformPoint } from '../util/misc/matrix'; import { isTransparent } from '../util/misc/isTransparent'; -import { TMat2D, TOriginX, TOriginY, TSize } from '../typedefs'; +import { AssertKeys, TMat2D, TOriginX, TOriginY, TSize } from '../typedefs'; import { degreesToRadians } from '../util/misc/radiansDegreesConversion'; import { getPointer, isTouchEvent } from '../util/dom_event'; import type { IText } from '../shapes/IText/IText'; @@ -1359,30 +1359,36 @@ export class SelectableCanvas< * Sets given object as the only active object on canvas * @param {FabricObject} object Object to set as an active one * @param {TPointerEvent} [e] Event (passed along when firing "object:selected") - * @chainable + * @return {Boolean} true if the object has been selected */ - setActiveObject(object: FabricObject, e?: TPointerEvent) { + setActiveObject( + object: FabricObject, + e?: TPointerEvent + ): this is AssertKeys { // we can't inline this, since _setActiveObject will change what getActiveObjects returns const currentActives = this.getActiveObjects(); - this._setActiveObject(object, e); + const selected = this._setActiveObject(object, e); this._fireSelectionEvents(currentActives, e); + return selected; } /** - * This is a private method for now. * This is supposed to be equivalent to setActiveObject but without firing * any event. There is commitment to have this stay this way. * This is the functional part of setActiveObject. - * @private * @param {Object} object to set as active * @param {Event} [e] Event (passed along when firing "object:selected") - * @return {Boolean} true if the selection happened + * @return {Boolean} true if the object has been selected */ - _setActiveObject(object: FabricObject, e?: TPointerEvent) { + _setActiveObject( + object: FabricObject, + e?: TPointerEvent + ): this is AssertKeys { if (this._activeObject === object) { return false; } - if (!this._discardActiveObject(e, object)) { + if (!this._discardActiveObject(e, object) && this._activeObject) { + // refused to deselect return false; } if (object.onSelect({ e })) { @@ -1394,16 +1400,17 @@ export class SelectableCanvas< } /** - * This is a private method for now. * This is supposed to be equivalent to discardActiveObject but without firing * any selection events ( can still fire object transformation events ). There is commitment to have this stay this way. * This is the functional part of discardActiveObject. * @param {Event} [e] Event (passed along when firing "object:deselected") * @param {Object} object the next object to set as active, reason why we are discarding this - * @return {Boolean} true if the selection happened - * @private + * @return {Boolean} true if the active object has been discarded */ - _discardActiveObject(e?: TPointerEvent, object?: FabricObject) { + _discardActiveObject( + e?: TPointerEvent, + object?: FabricObject + ): this is { _activeObject: undefined } { const obj = this._activeObject; if (obj) { // onDeselect return TRUE to cancel selection; @@ -1420,8 +1427,9 @@ export class SelectableCanvas< this.endCurrentTransform(e); } this._activeObject = undefined; + return true; } - return true; + return false; } /** @@ -1430,9 +1438,9 @@ export class SelectableCanvas< * sent to the fire function for the custom events. When used as a method the * e param does not have any application. * @param {event} e - * @chainable + * @return {Boolean} true if the active object has been discarded */ - discardActiveObject(e?: TPointerEvent) { + discardActiveObject(e?: TPointerEvent): this is { _activeObject: undefined } { const currentActives = this.getActiveObjects(), activeObject = this.getActiveObject(); if (currentActives.length) { @@ -1441,8 +1449,9 @@ export class SelectableCanvas< deselected: [activeObject!], }); } - this._discardActiveObject(e); + const discarded = this._discardActiveObject(e); this._fireSelectionEvents(currentActives, e); + return discarded; } /** @@ -1499,8 +1508,10 @@ export class SelectableCanvas< * Clears all contexts (background, main, top) of an instance */ clear() { - // this.discardActiveGroup(); + // discard active object and fire events this.discardActiveObject(); + // make sure we clear the active object in case it refused to be discarded + this._activeObject = undefined; this.clearContext(this.contextTop); super.clear(); } diff --git a/test/unit/canvas.js b/test/unit/canvas.js index 479ec3b34c3..1d27c26a043 100644 --- a/test/unit/canvas.js +++ b/test/unit/canvas.js @@ -1762,10 +1762,11 @@ canvas.add(rect1, rect2); - canvas.setActiveObject(rect1); + assert.ok(canvas.setActiveObject(rect1), 'selected'); assert.ok(rect1 === canvas._activeObject); - canvas.setActiveObject(rect2); + assert.ok(canvas.setActiveObject(rect2), 'selected'); + assert.ok(!canvas.setActiveObject(rect2), 'no effect'); assert.ok(rect2 === canvas._activeObject); }); @@ -1829,7 +1830,8 @@ canvas.add(makeRect()); canvas.setActiveObject(canvas.item(0)); - canvas._discardActiveObject(); + assert.ok(canvas._discardActiveObject(), 'discarded'); + assert.ok(!canvas._discardActiveObject(), 'no effect'); assert.equal(canvas.getActiveObject(), null); }); @@ -1868,8 +1870,9 @@ eventsFired.selectionCleared = true; }); - canvas.discardActiveObject(); - assert.equal(canvas.getActiveObject(), null); + assert.ok(canvas.discardActiveObject(), 'deselected'); + assert.ok(!canvas.getActiveObject(), 'no active object'); + assert.ok(!canvas.discardActiveObject(), 'no effect'); assert.equal(canvas.getActiveObject(), null); for (var prop in eventsFired) { @@ -1877,6 +1880,16 @@ } }); + QUnit.test('refuse discarding active object', function (assert) { + const rect = makeRect(); + rect.onDeselect = () => true; + canvas.setActiveObject(rect); + assert.ok(!canvas.discardActiveObject(), 'no effect'); + assert.ok(canvas.getActiveObject() === rect, 'active object'); + canvas.clear(); + assert.ok(!canvas.getActiveObject(), 'cleared the stubborn ref'); + }) + QUnit.test('complexity', function(assert) { assert.ok(typeof canvas.complexity === 'function'); assert.equal(canvas.complexity(), 0);