From 138b5924331c605e0a3435aee10f5ff46f300e99 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sat, 12 Feb 2022 01:45:31 +0200 Subject: [PATCH] try fixing selection logic --- src/canvas.class.js | 7 +- src/mixins/canvas_events.mixin.js | 10 +-- src/mixins/canvas_grouping.mixin.js | 114 ++++++++++++++------------- src/shapes/active_selection.class.js | 6 +- 4 files changed, 66 insertions(+), 71 deletions(-) diff --git a/src/canvas.class.js b/src/canvas.class.js index 7788b5acc76..850770952a0 100644 --- a/src/canvas.class.js +++ b/src/canvas.class.js @@ -647,7 +647,6 @@ scaleY: target.scaleY, skewX: target.skewX, skewY: target.skewY, - // used by transation offsetX: pointer.x - target.left, offsetY: pointer.y - target.top, originX: origin.x, @@ -656,11 +655,7 @@ ey: pointer.y, lastX: pointer.x, lastY: pointer.y, - // unsure they are useful anymore. - // left: target.left, - // top: target.top, theta: degreesToRadians(target.angle), - // end of unsure width: target.width * target.scaleX, shiftKey: e.shiftKey, altKey: altKey, @@ -753,7 +748,7 @@ if (shouldLookForActive && activeObject._findTargetCorner(pointer, isTouch)) { return activeObject; } - if (aObjects.length > 1 && !skipGroup && activeObject === this.searchPossibleTargets([activeObject], pointer)) { + if (aObjects.length > 1 && activeObject.type === 'activeSelection' && !skipGroup && this.searchPossibleTargets([activeObject], pointer)) { return activeObject; } if (aObjects.length === 1 && diff --git a/src/mixins/canvas_events.mixin.js b/src/mixins/canvas_events.mixin.js index 606d9cffedf..0d62061a067 100644 --- a/src/mixins/canvas_events.mixin.js +++ b/src/mixins/canvas_events.mixin.js @@ -671,13 +671,13 @@ // save pointer for check in __onMouseUp event this._previousPointer = pointer; var shouldRender = this._shouldRender(target), - shouldGroup = this._shouldGroup(e, target); + didGroup = false; if (this._shouldClearSelection(e, target)) { this.discardActiveObject(e); } - else if (shouldGroup) { - this._handleGrouping(e, target); + else if (this._handleGrouping(e, target)) { target = this._activeObject; + didGroup = true; } if (this.selection && (!target || @@ -700,7 +700,7 @@ fabric.util.isTouchEvent(e) ); target.__corner = corner; - if (target === this._activeObject && (corner || !shouldGroup)) { + if (target === this._activeObject && (corner || !didGroup)) { this._setupCurrentTransform(e, target, alreadySelected); var control = target.controls[corner], pointer = this.getPointer(e), @@ -712,7 +712,7 @@ } this._handleEvent(e, 'down'); // we must renderAll so that we update the visuals - (shouldRender || shouldGroup) && this.requestRenderAll(); + (shouldRender || didGroup) && this.requestRenderAll(); }, /** diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index b3775557277..a6236eb68df 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -9,49 +9,41 @@ * @private * @param {Event} e Event object * @param {fabric.Object} target - * @return {Boolean} - */ - _shouldGroup: function(e, target) { - var activeObject = this._activeObject; - return activeObject && this._isSelectionKeyPressed(e) && target && target.selectable && this.selection && - (activeObject !== target || activeObject.type === 'activeSelection') && !target.onSelect({ e: e }); - }, - - /** - * @private - * @param {Event} e Event object - * @param {fabric.Object} target + * @returns {boolean} true if grouping occured */ _handleGrouping: function (e, target) { var activeObject = this._activeObject; + if (!(activeObject && this._isSelectionKeyPressed(e) && this.selection && target && target.selectable && !target.onSelect({ e: e }))) { + return false; + } // avoid multi select when shift click on a corner if (activeObject.__corner) { - return; + return false; } if (target === activeObject) { - // if it's a group, find target again, using activeGroup objects - target = this.findTarget(e, true); - // if even object is not found or we are on activeObjectCorner, bail out + target = this.targets.pop(); if (!target || !target.selectable) { - return; + return false; } } - if (activeObject && activeObject.type === 'activeSelection') { - this._updateActiveSelection(target, e); - } - else { + return activeObject && activeObject.type === 'activeSelection' ? + this._updateActiveSelection(target, e) : this._createActiveSelection(target, e); - } }, /** * @private + * @returns {boolean} true if target was added to active selection */ _updateActiveSelection: function(target, e) { var activeSelection = this._activeObject, - currentActiveObjects = activeSelection._objects.slice(0); - if (activeSelection.contains(target)) { + currentActiveObjects = activeSelection._objects.slice(0), + modified = false; + // an object is about to be removed from active selection + // we make sure it is a direct child of active selection + if (target.group === activeSelection) { activeSelection.removeWithUpdate(target); + modified = true; this._hoveredTarget = target; this._hoveredTargets = this.targets.concat(); if (activeSelection.size() === 1) { @@ -59,18 +51,29 @@ this._setActiveObject(activeSelection.item(0), e); } } - else { + // an object is about to be added to active selection + // we make sure it is not a already a descendant of active selection + else if (!target.isDescendantOf(activeSelection)) { activeSelection.addWithUpdate(target); + modified = true; this._hoveredTarget = activeSelection; this._hoveredTargets = this.targets.concat(); } - this._fireSelectionEvents(currentActiveObjects, e); + modified && this._fireSelectionEvents(currentActiveObjects, e); + return modified; }, /** * @private + * @returns {boolean} true if active selection was created */ - _createActiveSelection: function(target, e) { + _createActiveSelection: function (target, e) { + var activeObject = this._activeObject; + // target is about be join active selection + // we make sure objects aren't ancestors of each other in order to avoid recursive selection + if (target === activeObject || target.isDescendantOf(activeObject) || activeObject.isDescendantOf(target)) { + return false; + } var currentActives = this.getActiveObjects(), group = this._createGroup(target); this._hoveredTarget = group; // ISSUE 4115: should we consider subTargets here? @@ -78,45 +81,25 @@ // this._hoveredTargets = this.targets.concat(); this._setActiveObject(group, e); this._fireSelectionEvents(currentActives, e); + return true; }, /** * @private * @param {Object} target */ - _createGroup: function(target) { - var objects = this._objects, - isActiveLower = objects.indexOf(this._activeObject) < objects.indexOf(target), - groupObjects = isActiveLower - ? [this._activeObject, target] - : [target, this._activeObject]; - this._activeObject.isEditing && this._activeObject.exitEditing(); + _createGroup: function (target) { + var activeObject = this._activeObject; + var groupObjects = activeObject.isInFrontOf(target) ? + [activeObject, target] : + [target, activeObject]; + activeObject.isEditing && activeObject.exitEditing(); + // handle case: target is nested return new fabric.ActiveSelection(groupObjects, { canvas: this }); }, - /** - * @private - * @param {Event} e mouse event - */ - _groupSelectedObjects: function (e) { - - var group = this._collectObjects(e), - aGroup; - - // do not create group for 1 element only - if (group.length === 1) { - this.setActiveObject(group[0], e); - } - else if (group.length > 1) { - aGroup = new fabric.ActiveSelection(group.reverse(), { - canvas: this - }); - this.setActiveObject(aGroup, e); - } - }, - /** * @private */ @@ -161,6 +144,27 @@ return group; }, + /** + * @private + * @param {Event} e mouse event + */ + _groupSelectedObjects: function (e) { + + var objects = this._collectObjects(e), + aGroup; + + // do not create group for 1 element only + if (objects.length === 1) { + this.setActiveObject(objects[0], e); + } + else if (objects.length > 1) { + aGroup = new fabric.ActiveSelection(objects.reverse(), { + canvas: this + }); + this.setActiveObject(aGroup, e); + } + }, + /** * @private */ diff --git a/src/shapes/active_selection.class.js b/src/shapes/active_selection.class.js index d476a3dbf63..8c0c9eaba2e 100644 --- a/src/shapes/active_selection.class.js +++ b/src/shapes/active_selection.class.js @@ -24,11 +24,6 @@ */ type: 'activeSelection', - /** - * disabled for proper functionality - */ - subTargetCheck: false, - /** * Constructor * @param {Object} objects ActiveSelection objects @@ -51,6 +46,7 @@ this._calcBounds(); this._updateObjectsCoords(); fabric.Object.prototype.initialize.call(this, options); + this._updateObjectsCoords(); this.setCoords(); },