diff --git a/src/mixins/collection.mixin.js b/src/mixins/collection.mixin.js index 1e91693fc4c..252ca8c96ef 100644 --- a/src/mixins/collection.mixin.js +++ b/src/mixins/collection.mixin.js @@ -34,6 +34,7 @@ fabric.Collection = { * @param {fabric.Object|fabric.Object[]} objects Object(s) to insert * @param {Number} index Index to insert object at * @param {(object:fabric.Object) => any} [callback] + * @returns {number} new array length */ insertAt: function (objects, index, callback) { var args = [index, 0].concat(objects); @@ -43,6 +44,7 @@ fabric.Collection = { callback.call(this, args[i]); } } + return this._objects.length; }, /** diff --git a/src/shapes/active_selection.class.js b/src/shapes/active_selection.class.js index 4d64b2aaa19..8d4c27a1fa8 100644 --- a/src/shapes/active_selection.class.js +++ b/src/shapes/active_selection.class.js @@ -66,9 +66,6 @@ * @returns {boolean} true if object entered group */ enterGroup: function (object, removeParentTransform) { - if (!this.canEnter(object)) { - return false; - } if (object.group) { // save ref to group for later in order to return to it var parent = object.group; diff --git a/src/shapes/group.class.js b/src/shapes/group.class.js index e26412d6d91..f04a0723705 100644 --- a/src/shapes/group.class.js +++ b/src/shapes/group.class.js @@ -141,8 +141,12 @@ * @param {...fabric.Object} objects */ add: function () { - fabric.Collection.add.call(this, arguments, this._onObjectAdded); - this._onAfterObjectsChange('added', Array.from(arguments)); + var _this = this, possibleObjects = Array.from(arguments).filter(function(object, index, array) { + // can enter or is the first occurrence of the object in the passed args + return _this.canEnterGroup(object) && array.indexOf(object) === index; + }); + fabric.Collection.add.call(this, possibleObjects, this._onObjectAdded); + this._onAfterObjectsChange('added', possibleObjects); }, /** @@ -226,16 +230,17 @@ * @param {fabric.Object} object * @returns */ - canEnter: function (object) { + canEnterGroup: function (object) { if (object === this || this.isDescendantOf(object)) { /* _DEV_MODE_START_ */ - console.warn('fabric.Group: trying to add group to itself, this call has no effect'); + console.error('fabric.Group: trying to add group to itself, this call has no effect'); /* _DEV_MODE_END_ */ return false; } - else if (object.group && object.group === this) { + else if (this._objects.indexOf(object) !== -1) { + // is already in the objects array /* _DEV_MODE_START_ */ - console.warn('fabric.Group: duplicate objects are not supported inside group, this call has no effect'); + console.error('fabric.Group: duplicate objects are not supported inside group, this call has no effect'); /* _DEV_MODE_END_ */ return false; } @@ -249,9 +254,6 @@ * @returns {boolean} true if object entered group */ enterGroup: function (object, removeParentTransform) { - if (!this.canEnter(object)) { - return false; - } if (object.group) { object.group.remove(object); } @@ -591,7 +593,7 @@ return this.prepareBoundingBox(layoutDirective, addedObjects, context); } else if (layoutDirective === 'fit-content' || layoutDirective === 'fit-content-lazy' - || (layoutDirective === 'fixed' && context.type === 'initialization')) { + || (layoutDirective === 'fixed' && (context.type === 'initialization' || context.type === 'imperative'))) { return this.prepareBoundingBox(layoutDirective, objects, context); } else if (layoutDirective === 'clip-path' && this.clipPath) { @@ -872,6 +874,7 @@ this._watchObject(false, object); object.dispose && object.dispose(); }, this); + this.callSuper('dispose'); }, /* _TO_SVG_START_ */ diff --git a/src/static_canvas.class.js b/src/static_canvas.class.js index ca254eadb6e..d0a37e94a41 100644 --- a/src/static_canvas.class.js +++ b/src/static_canvas.class.js @@ -586,7 +586,7 @@ */ insertAt: function (objects, index) { fabric.Collection.insertAt.call(this, objects, index, this._onObjectAdded); - this.renderOnAddRemove && this.requestRenderAll(); + (Array.isArray(objects) ? objects.length > 0 : !!objects) && this.renderOnAddRemove && this.requestRenderAll(); return this; }, diff --git a/test/unit/group.js b/test/unit/group.js index b3183c00058..f744269efa1 100644 --- a/test/unit/group.js +++ b/test/unit/group.js @@ -702,25 +702,25 @@ group = new fabric.Group([rect1]); // duplicate - assert.notOk(group.canEnter(rect1)); - // group.add(rect1); - // assert.deepEqual(group.getObjects(), [rect1], 'objects should not have changed'); + assert.notOk(group.canEnterGroup(rect1)); + group.add(rect1); + assert.deepEqual(group.getObjects(), [rect1], 'objects should not have changed'); // duplicate on same call - assert.ok(group.canEnter(rect2)); - // group.add(rect2, rect2); - // assert.deepEqual(group.getObjects(), [rect1, rect2], '`rect2` should have entered once'); + assert.ok(group.canEnterGroup(rect2)); + group.add(rect2, rect2); + assert.deepEqual(group.getObjects(), [rect1, rect2], '`rect2` should have entered once'); // adding self - assert.notOk(group.canEnter(group)); - // group.add(group); - // assert.deepEqual(group.getObjects(), [rect1, rect2], 'objects should not have changed'); + assert.notOk(group.canEnterGroup(group)); + group.add(group); + assert.deepEqual(group.getObjects(), [rect1, rect2], 'objects should not have changed'); // nested object should be removed from group var nestedGroup = new fabric.Group([rect1]); - assert.ok(group.canEnter(nestedGroup)); - // group.add(nestedGroup); - // assert.deepEqual(group.getObjects(), [rect2, nestedGroup], '`rect1` was removed from group once it entered `nestedGroup`'); + assert.ok(group.canEnterGroup(nestedGroup)); + group.add(nestedGroup); + assert.deepEqual(group.getObjects(), [rect2, nestedGroup], '`rect1` was removed from group once it entered `nestedGroup`'); // circular group var circularGroup = new fabric.Group([rect2, group]); - assert.notOk(group.canEnter(circularGroup), 'circular group should be denied entry'); + assert.notOk(group.canEnterGroup(circularGroup), 'circular group should be denied entry'); // group.add(circularGroup); // assert.deepEqual(group.getObjects(), [rect2, nestedGroup], 'objects should not have changed'); });