Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

Closed
wants to merge 106 commits into from
Closed
Show file tree
Hide file tree
Changes from 104 commits
Commits
Show all changes
106 commits
Select commit Hold shift + click to select a range
1a191a7
apply 7744 patch (no binaries)
ShaMan123 Apr 3, 2022
26e8392
apply 7744 patch (binaries)
ShaMan123 Apr 3, 2022
7d793b3
commit #7744 review
ShaMan123 Apr 3, 2022
2708029
patch2
ShaMan123 Apr 4, 2022
02a8ffa
Update spray_brush.class.js
ShaMan123 Apr 4, 2022
0d3690f
enable text editing under group
ShaMan123 Apr 4, 2022
ff0346a
migrate back to isDescendantOf
ShaMan123 Apr 4, 2022
812f02a
render bg
ShaMan123 Apr 4, 2022
053946c
fix(): clear canvas from entire object tree
ShaMan123 Apr 4, 2022
db76c44
fix(): center object in case object is nested
ShaMan123 Apr 4, 2022
04aad5b
Update eraser_brush.mixin.js
ShaMan123 Apr 4, 2022
8c1c535
Update canvas.class.js
ShaMan123 Apr 4, 2022
3fd4af2
Update canvas_events.mixin.js
ShaMan123 Apr 4, 2022
58e9be2
lint
ShaMan123 Apr 4, 2022
c2ef2f5
safeguard from adding group to itself
ShaMan123 Apr 4, 2022
f0145b5
Update object_ancestry.mixin.js
ShaMan123 Apr 4, 2022
cb20b81
patch tests from v6!
ShaMan123 Apr 4, 2022
347f80d
Update canvas_grouping.mixin.js
ShaMan123 Apr 4, 2022
a56d55e
Update canvas.class.js
ShaMan123 Apr 4, 2022
2a426e7
Update group_layout.js
ShaMan123 Apr 4, 2022
5e3fc1f
Update group_layout.js
ShaMan123 Apr 4, 2022
2b1f680
remove redundant logic
ShaMan123 Apr 4, 2022
1ad5cc8
Update canvas_grouping.mixin.js
ShaMan123 Apr 4, 2022
11c828f
fix(): calling sendPointToPlane
ShaMan123 Apr 4, 2022
b6fa3dc
Update group.class.js
ShaMan123 Apr 4, 2022
e9afc89
refactor `enterGroup`
ShaMan123 Apr 4, 2022
d8f8251
invalidate groups after active selection change
ShaMan123 Apr 4, 2022
c58a274
_shouldSetNestedCoords
ShaMan123 Apr 4, 2022
c0f5571
perf(): refrain from `contains`
ShaMan123 Apr 4, 2022
9a223d0
Update canvas_events.mixin.js
ShaMan123 Apr 4, 2022
2d5cc75
Update canvas.class.js
ShaMan123 Apr 4, 2022
01c4a87
Update canvas.class.js
ShaMan123 Apr 4, 2022
2164161
fix(): export svg backgroundColor
ShaMan123 Apr 4, 2022
a5575bd
Update group.js
ShaMan123 Apr 4, 2022
5c56124
backport collection fix to test
ShaMan123 Apr 4, 2022
5e3aaf5
Update group.class.js
ShaMan123 Apr 4, 2022
d53d6b6
lint
ShaMan123 Apr 4, 2022
8e907bb
fix(): target prop on canvas added/removed event
ShaMan123 Apr 5, 2022
f082fc9
fix(): modify handlers strictly on canvas events
ShaMan123 Apr 5, 2022
c2d93db
Merge branch 'master' into v6-group-patch2
ShaMan123 Apr 5, 2022
2f346b0
Update group_layout.js
ShaMan123 Apr 5, 2022
9931a3c
Merge branch 'master' into v6-group-patch2
ShaMan123 Apr 5, 2022
d6f1950
Merge branch 'master' into v6-group-patch1
ShaMan123 Apr 5, 2022
5cb64af
Merge branch 'v6-group-patch1' into v6-group-patch2
ShaMan123 Apr 5, 2022
63cab25
Merge branch 'master' into v6-group-patch2
asturur Apr 7, 2022
21110d3
Update spray_brush.class.js
ShaMan123 Apr 8, 2022
6c141e1
remove merge error
ShaMan123 Apr 9, 2022
465ea96
add some comment
asturur Apr 9, 2022
ffafe66
more comments
asturur Apr 9, 2022
e2d03f0
only a comment and removed an extra negation by swapping a ternary
asturur Apr 9, 2022
3e09114
better invalidation + comment
ShaMan123 Apr 10, 2022
af3430c
fix(): text enterEditing in interactive group
ShaMan123 Apr 10, 2022
67abe2f
fix(): strict ancestors
ShaMan123 Apr 10, 2022
b8477f2
test `findCommonAncestors`
ShaMan123 Apr 10, 2022
d039d04
add strict option + fixes
ShaMan123 Apr 10, 2022
2c49f67
Revert "fix(): strict ancestors"
ShaMan123 Apr 10, 2022
7ba7fa4
fix test
ShaMan123 Apr 10, 2022
de96660
better comments
ShaMan123 Apr 10, 2022
b566158
better comment
ShaMan123 Apr 10, 2022
beaa331
JSDOC
ShaMan123 Apr 10, 2022
c0aea33
remove redundant `len` variable
ShaMan123 Apr 10, 2022
3ec53b4
fix(Collection): `remove` return value
ShaMan123 Apr 10, 2022
1c414cf
typo
ShaMan123 Apr 10, 2022
2c65a5c
adjust tests
ShaMan123 Apr 10, 2022
785433a
Update group.class.js
ShaMan123 Apr 10, 2022
56a2b73
Update collection.mixin.js
ShaMan123 Apr 10, 2022
3df9435
fix(): safegurad from circular refs
ShaMan123 Apr 10, 2022
d49baac
Update group.js
ShaMan123 Apr 10, 2022
31d2107
Update group.js
ShaMan123 Apr 10, 2022
3f837ce
refactor `_chooseObjectsToRender`
ShaMan123 Apr 18, 2022
dd9f194
fix(): `_activeObjects`
ShaMan123 Apr 18, 2022
014de45
Update active_selection.class.js
ShaMan123 Apr 18, 2022
9a48c10
fix(): render && preserveObjectStacking
ShaMan123 Apr 18, 2022
51a454c
Update group.class.js
ShaMan123 Apr 18, 2022
020c2bb
preserve active selection ref
ShaMan123 Apr 18, 2022
6d0ed73
fix(): propagate `dirty` up to owning group
ShaMan123 Apr 18, 2022
d18351f
move
ShaMan123 Apr 18, 2022
de46f45
Update canvas.class.js
ShaMan123 Apr 18, 2022
a456afe
Revert "move"
ShaMan123 Apr 18, 2022
9214d8b
fix(): remove redundant transform
ShaMan123 Apr 18, 2022
9665b54
test dirty propagation
ShaMan123 Apr 18, 2022
fba43bd
fix(): propagate dirty flag + `preserveObjectStacking`
ShaMan123 Apr 18, 2022
8c095a3
Update activeselection.js
ShaMan123 Apr 18, 2022
11da63b
Update activeselection.js
ShaMan123 Apr 18, 2022
6d5df90
Update activeselection.js
ShaMan123 Apr 18, 2022
8377693
fix(): `_chooseObjectsToRender` single active obj
ShaMan123 Apr 18, 2022
08f3c83
safety
ShaMan123 Apr 18, 2022
e23471c
remove dead test
ShaMan123 Apr 18, 2022
b9a32d5
feat(): invalidate group + `progress` layout
ShaMan123 Apr 18, 2022
dbc2e7d
Update active_selection.class.js
ShaMan123 Apr 18, 2022
f34ccb3
Revert "Update active_selection.class.js"
ShaMan123 Apr 18, 2022
0fc26e3
disable `progress` layouts
ShaMan123 Apr 18, 2022
f642a28
Update group.class.js
ShaMan123 Apr 18, 2022
2c78fb5
lint
ShaMan123 Apr 18, 2022
245a796
Update util.js
ShaMan123 Apr 18, 2022
84c3345
Update canvas_grouping.mixin.js
ShaMan123 Apr 18, 2022
79db7d5
Update object.class.js
ShaMan123 Apr 18, 2022
24c279f
add comment
ShaMan123 Apr 18, 2022
91232c8
tests
ShaMan123 Apr 18, 2022
7b479ca
Update group_layout.js
ShaMan123 Apr 18, 2022
fc48a27
Update group_layout.js
ShaMan123 Apr 18, 2022
9d77261
Update group_layout.js
ShaMan123 Apr 18, 2022
5f3a39f
remove foul `isCacheDirty`
ShaMan123 Apr 18, 2022
864f174
Merge branch 'master' into v6-nested-selection
ShaMan123 May 1, 2022
8c317fe
Merge branch 'master' into v6-nested-selection
ShaMan123 May 2, 2022
91a7f53
Merge branch 'master' into v6-nested-selection
ShaMan123 Jun 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 16 additions & 31 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@
_initInteractive: function() {
this._currentTransform = null;
this._groupSelector = null;
this._activeSelection = new fabric.ActiveSelection([], { canvas: this });
this._initWrapperElement();
this._createUpperCanvas();
this._initEventListeners();
Expand Down Expand Up @@ -420,40 +421,16 @@
* and one to render as activeGroup.
* @return {Array} objects to render immediately and pushes the other in the activeGroup.
*/
_chooseObjectsToRender: function() {
var activeObjects = this.getActiveObjects(),
object, objsToRender, activeGroupObjects;

if (!this.preserveObjectStacking && activeObjects.length > 1) {
objsToRender = [];
activeGroupObjects = [];
for (var i = 0, length = this._objects.length; i < length; i++) {
object = this._objects[i];
if (activeObjects.indexOf(object) === -1 ) {
objsToRender.push(object);
}
else {
activeGroupObjects.push(object);
}
}
if (activeObjects.length > 1) {
this._activeObject._objects = activeGroupObjects;
}
objsToRender.push.apply(objsToRender, activeGroupObjects);
}
// in case a single object is selected render it's entire parent above the other objects
else if (!this.preserveObjectStacking && activeObjects.length === 1) {
var target = activeObjects[0], ancestors = target.getAncestors(true);
var topAncestor = ancestors.length === 0 ? target : ancestors.pop();
objsToRender = this._objects.slice();
var index = objsToRender.indexOf(topAncestor);
index > -1 && objsToRender.splice(objsToRender.indexOf(topAncestor), 1);
objsToRender.push(topAncestor);
_chooseObjectsToRender: function () {
var activeObject = this._activeObject;
if (!this.preserveObjectStacking && activeObject) {
return this._objects.filter(function (object) {
return !object.group && object !== activeObject;
}).concat(activeObject);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can stop this foolishness by doing what #8034 does on Group
I didn't touch ActiveSelection because of this piece of logic but I don't see a reason why not if we want to drop this ugly part.

}
else {
objsToRender = this._objects;
return this._objects;
}
return objsToRender;
},

/**
Expand Down Expand Up @@ -1116,6 +1093,14 @@
return [];
},

/**
* Returns instance's active selection
* @return {fabric.ActiveSelection} active selection
*/
getActiveSelection: function () {
return this._activeSelection;
},

/**
* @private
* @param {fabric.Object} obj Object that was removed
Expand Down
25 changes: 10 additions & 15 deletions src/mixins/canvas_grouping.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,30 +103,25 @@
[activeObject, target] :
[target, activeObject];
activeObject.isEditing && activeObject.exitEditing();
// handle case: target is nested
return new fabric.ActiveSelection(groupObjects, {
canvas: this
});
this._activeSelection.removeAll();
fabric.util.resetObjectTransform(this._activeSelection);
this._activeSelection.add.apply(this._activeSelection, groupObjects);
return this._activeSelection;
},

/**
* @private
* @param {Event} e mouse event
*/
_groupSelectedObjects: function (e) {

var group = this._collectObjects(e),
aGroup;

var objects = this._collectObjects(e);
// do not create group for 1 element only
if (group.length === 1) {
this.setActiveObject(group[0], e);
if (objects.length === 1) {
this.setActiveObject(objects[0], e);
}
else if (group.length > 1) {
aGroup = new fabric.ActiveSelection(group.reverse(), {
canvas: this
});
this.setActiveObject(aGroup, e);
else if (objects.length > 1) {
this._activeSelection.add.apply(this._activeSelection, objects.reverse());
this.setActiveObject(this._activeSelection, e);
}
},

Expand Down
48 changes: 46 additions & 2 deletions src/shapes/active_selection.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,49 @@
this.setCoords();
},

/**
* @private
* @param {string} key
* @param {*} value
*/
_set: function (key, value) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the only part of active selection that hasn't been ported to #8665

var prev = this[key];
this.callSuper('_set', key, value);
if (prev !== this[key] && this.canvas && this.canvas.preserveObjectStacking
&& this.stateProperties.indexOf(key) > -1) {
var invalidationContext = {
target: this,
key: key,
value: value,
prevValue: prev
};
var invalidatedGroups = [];
this.forEachObject(function (object) {
var group = object.__owningGroup;
if (group && invalidatedGroups.indexOf(group) === -1) {
group.invalidate(invalidationContext);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe I want to add a check for objects that are group de-facto but aren't really groups:

Suggested change
group.invalidate(invalidationContext);
group.invalidate && group.invalidate(invalidationContext);

invalidatedGroups.push(group);
}
});
}
return this;
},

/**
* @private
*/
_shouldSetNestedCoords: function () {
return true;
},

/**
* @private
* @override we don't want the selection monitor to be active
*/
__objectSelectionMonitor: function () {
// noop
},

/**
* @private
* @param {fabric.Object} object
Expand All @@ -72,7 +108,6 @@
if (object.group) {
// save ref to group for later in order to return to it
var parent = object.group;
parent._exitGroup(object);
object.__owningGroup = parent;
}
this._enterGroup(object, removeParentTransform);
Expand All @@ -90,7 +125,7 @@
var parent = object.__owningGroup;
if (parent) {
// return to owning group
parent.enterGroup(object);
parent._enterGroup(object, true);
delete object.__owningGroup;
}
},
Expand All @@ -101,6 +136,7 @@
* @param {fabric.Object[]} targets
*/
_onAfterObjectsChange: function (type, targets) {
this.callSuper('_onAfterObjectsChange', type, targets);
var groups = [];
targets.forEach(function (object) {
object.group && !groups.includes(object.group) && groups.push(object.group);
Expand All @@ -119,6 +155,14 @@
}
},

/**
* @override there's no reason to invalidate because objects aren't selectable
* @private
*/
invalidate: function () {
// noop
},

/**
* If returns true, deselection is cancelled.
* @since 2.0.0
Expand Down
82 changes: 49 additions & 33 deletions src/shapes/group.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@
object.group.remove(object);
}
this._enterGroup(object, removeParentTransform);
var activeObject = this.canvas && this.canvas.getActiveObject && this.canvas.getActiveObject();
// if we are adding the activeObject in a group
if (activeObject && (activeObject === object || object.isDescendantOf(activeObject))) {
this._activeObjects.push(object);
}
Comment on lines +260 to +264
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this part....\

return true;
},

Expand All @@ -279,11 +284,6 @@
object._set('group', this);
object._set('canvas', this.canvas);
this.interactive && this._watchObject(true, object);
var activeObject = this.canvas && this.canvas.getActiveObject && this.canvas.getActiveObject();
// if we are adding the activeObject in a group
if (activeObject && (activeObject === object || object.isDescendantOf(activeObject))) {
this._activeObjects.push(object);
}
},

/**
Expand Down Expand Up @@ -409,35 +409,23 @@
* Execute the drawing operation for an object on a specified context
* @param {CanvasRenderingContext2D} ctx Context to render on
*/
drawObject: function(ctx) {
drawObject: function (ctx) {
this._renderBackground(ctx);
for (var i = 0; i < this._objects.length; i++) {
this._objects[i].render(ctx);
}
this._drawClipPath(ctx, this.clipPath);
},

/**
* Check if cache is dirty
*/
isCacheDirty: function(skipCanvas) {
if (this.callSuper('isCacheDirty', skipCanvas)) {
return true;
}
if (!this.statefullCache) {
return false;
}
for (var i = 0; i < this._objects.length; i++) {
if (this._objects[i].isCacheDirty(true)) {
if (this._cacheCanvas) {
// if this group has not a cache canvas there is nothing to clean
var x = this.cacheWidth / this.zoomX, y = this.cacheHeight / this.zoomY;
this._cacheContext.clearRect(-x / 2, -y / 2, x, y);
}
return true;
var preserveObjectStacking = this.canvas && this.canvas.preserveObjectStacking;
for (var i = 0, object; i < this._objects.length; i++) {
object = this._objects[i];
if (preserveObjectStacking && object.group !== this) {
// object is part of ActiveSelection
ctx.save();
ctx.transform.apply(ctx, invertTransform(this.calcTransformMatrix()));
object.render(ctx);
ctx.restore();
}
else if (preserveObjectStacking || (object.group === this && this._activeObjects.indexOf(object) === -1)) {
object.render(ctx);
}
}
return false;
this._drawClipPath(ctx, this.clipPath);
},

/**
Expand All @@ -462,6 +450,24 @@
this._transformDone = false;
},

/**
* @typedef {object} InvalidationContext
* @property {fabric.Object} target either a child object or {@link fabric.ActiveSelection}
* @property {string} key
* @property {*} value
* @property {*} prevValue
*
* @private
* @param {InvalidationContext} context
*/
invalidate: function (context) {
this.isOnACache() && (!this.canvas || this.canvas.preserveObjectStacking)
&& this._set('dirty', true);
this._applyLayoutStrategy(Object.assign({}, context, {
type: 'progress'
}));
},

/**
* @public
* @param {Partial<LayoutResult> & { layout?: string }} [context] pass values to use for layout calculations
Expand Down Expand Up @@ -500,9 +506,14 @@
// reject layout requests before initialization layout
return;
}
else if (context.type === 'progress' && this._layoutInProgress) {
// prevent circular calls
return;
}
var center = this.getRelativeCenterPoint();
var result = this.getLayoutStrategyResult(this.layout, this._objects.concat(), context);
if (result) {
this._layoutInProgress = true;
// handle positioning
var newCenter = new fabric.Point(result.centerX, result.centerY);
var vector = center.subtract(newCenter).add(new fabric.Point(result.correctionX || 0, result.correctionY || 0));
Expand All @@ -511,7 +522,7 @@
this.set({ width: result.width, height: result.height });
// adjust objects to account for new center
!context.objectsRelativeToGroup && this.forEachObject(function (object) {
this._adjustObjectPosition(object, diff);
object.group === this && this._adjustObjectPosition(object, diff);
}, this);
// clip path as well
!isFirstLayout && this.layout !== 'clip-path' && this.clipPath && !this.clipPath.absolutePositioned
Expand All @@ -521,6 +532,7 @@
this.setPositionByOrigin(newCenter, 'center', 'center');
this.setCoords();
}
this._layoutInProgress = false;
}
else if (isFirstLayout) {
// fill `result` with initial values for the layout hook
Expand Down Expand Up @@ -582,9 +594,13 @@
* @returns {LayoutResult | undefined}
*/
getLayoutStrategyResult: function (layoutDirective, objects, context) { // eslint-disable-line no-unused-vars
if (context.type === 'progress'
/* && layoutDirective !== 'fit-content-lazy' && layoutDirective !== 'fit-content'*/) {
return;
}
// `fit-content-lazy` performance enhancement
// skip if instance had no objects before the `added` event because it may have kept layout after removing all previous objects
if (layoutDirective === 'fit-content-lazy'
else if (layoutDirective === 'fit-content-lazy'
&& context.type === 'added' && objects.length > context.targets.length) {
// calculate added objects' bbox with existing bbox
var addedObjects = context.targets.concat(this);
Expand Down
Loading