fix(Group): patch2 minors

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;
   },
 
   /**

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;

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_ */

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;
     },
 

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');
   });