fix(ActiveSelection): 🔃 preserveObjectStacking 📌

src/mixins/canvas_grouping.mixin.js

@@ -71,7 +71,14 @@
         }
       }
       else {
-        activeSelection.add(target);
+        //  preserveObjectStacking in ActiveSelection
+        //  perf enhancement for large ActiveSelection: consider a binary search of `isInFrontOf`
+        var insertAt = activeSelection._objects.findIndex(function (obj) {
+          return obj.isInFrontOf(target);
+        });
+        //  target might be in front of all other objects
+        insertAt = insertAt === -1 ? activeSelection._objects.length : insertAt;
+        activeSelection.insertAt(target, insertAt);
         this._hoveredTarget = activeSelection;
         this._hoveredTargets = this.targets.concat();
       }

src/mixins/object_ancestry.mixin.js

@@ -22,18 +22,32 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
   },
 
   /**
-   *
+   * Returns instance's parent **EXCLUDING** `ActiveSelection`
+   * @param {boolean} [strict] exclude canvas as well
+   * @returns {fabric.Object | fabric.StaticCanvas | undefined}
+   */
+  getParent: function (strict) {
+    return (
+      this.group && this.group.type === 'activeSelection' ?
+        this.__owningGroup :
+        this.group
+    ) || (strict ? undefined : this.canvas);
+  },
+
+  /**
+   * Returns an array of ancestors **EXCLUDING** `ActiveSelection`
+   * 
    * @typedef {fabric.Object[] | [...fabric.Object[], fabric.StaticCanvas]} Ancestors
-   *
+   * 
    * @param {boolean} [strict] returns only ancestors that are objects (without canvas)
    * @returns {Ancestors} ancestors from bottom to top
    */
   getAncestors: function (strict) {
     var ancestors = [];
-    var parent = this.group || (strict ? undefined : this.canvas);
+    var parent = this.getParent(strict);
     while (parent) {
       ancestors.push(parent);
-      parent = parent.group || (strict ? undefined : parent.canvas);
+      parent = parent.getParent && parent.getParent(strict);
     }
     return ancestors;
   },

src/shapes/active_selection.class.js

@@ -98,6 +98,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);

src/shapes/group.class.js

@@ -20,6 +20,8 @@
    * @class fabric.Group
    * @extends fabric.Object
    * @mixes fabric.Collection
+   * @fires object:added
+   * @fires object:removed
    * @fires layout once layout completes
    * @see {@link fabric.Group#initialize} for constructor definition
    */
@@ -352,6 +354,7 @@
      */
     _onObjectAdded: function (object) {
       this.enterGroup(object, true);
+      this.fire('object:added', { target: object });
       object.fire('added', { target: this });
     },
 
@@ -361,6 +364,7 @@
      */
     _onRelativeObjectAdded: function (object) {
       this.enterGroup(object, false);
+      this.fire('object:added', { target: object });
       object.fire('added', { target: this });
     },
 
@@ -371,6 +375,7 @@
      */
     _onObjectRemoved: function (object, removeParentTransform) {
       this.exitGroup(object, removeParentTransform);
+      this.fire('object:removed', { target: object });
       object.fire('removed', { target: this });
     },
 

test/unit/canvas.js

@@ -453,6 +453,56 @@
     assert.equal(selection.getObjects().indexOf(rect2), 1, 'rect2 is the second object in the active selection');
   });
 
+  QUnit.test('update active selection respects order of objects', function (assert) {
+    var rect1 = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    var rect3 = new fabric.Rect();
+    var rect3 = new fabric.Rect();
+    canvas.add(rect1, rect2, rect3);
+
+    function assertObjectsInOrder(init, added) {
+      var activeSelection = new fabric.ActiveSelection(init);
+      canvas.setActiveObject(activeSelection);
+      canvas._updateActiveSelection(added, {});
+      assert.deepEqual(canvas.getActiveObjects(), [rect1, rect2, rect3]);
+      activeSelection.removeAll();
+    }
+
+    function assertObjectsInOrderOnCanvas(init, added) {
+      assert.deepEqual(canvas.getObjects(), [rect1, rect2, rect3]);
+      assertObjectsInOrder(init, added);
+      assert.deepEqual(canvas.getObjects(), [rect1, rect2, rect3]);
+    }
+
+    assertObjectsInOrderOnCanvas([rect1, rect2], rect3);
+    assertObjectsInOrderOnCanvas([rect1, rect3], rect2);
+    assertObjectsInOrderOnCanvas([rect2, rect3], rect1);
+
+    canvas.remove(rect2, rect3);
+    var group = new fabric.Group([rect2, rect3], { subTargetCheck: true, interactive: true });
+    canvas.add(group);
+
+    function assertNestedObjectsInOrder(init, added) {
+      assert.deepEqual(canvas.getObjects(), [rect1, group]);
+      assert.deepEqual(group.getObjects(), [rect2, rect3]);
+      assertObjectsInOrder(init, added);
+      assert.deepEqual(canvas.getObjects(), [rect1, group]);
+      assert.deepEqual(group.getObjects(), [rect2, rect3]);
+    }
+
+    assertNestedObjectsInOrder([rect1, rect2], rect3);
+    assertNestedObjectsInOrder([rect1, rect3], rect2);
+    assertNestedObjectsInOrder([rect2, rect3], rect1);
+
+    canvas.remove(rect1);
+    group.insertAt(rect1, 0);
+    group.remove(rect3);
+    canvas.add(rect3);
+    assertNestedObjectsInOrder([rect1, rect2], rect3);
+    assertNestedObjectsInOrder([rect1, rect3], rect2);
+    assertNestedObjectsInOrder([rect2, rect3], rect1);
+  });
+
   QUnit.test('_groupSelectedObjects fires selected for objects', function(assert) {
     var fired = 0;
     var rect1 = new fabric.Rect();

test/unit/group.js

@@ -112,13 +112,14 @@
     });
     var removed = group.remove(rect2);
     assert.deepEqual(removed, [rect2], 'should return removed objects');
+    assert.deepEqual(targets, [rect2], 'should contain removed objects');
     assert.deepEqual(group.getObjects(), [rect1, rect3], 'should remove object properly');
 
     var removed = group.remove(rect1, rect3);
     assert.deepEqual(removed, [rect1, rect3], 'should return removed objects');
     assert.equal(group.isEmpty(), true, 'group should be empty');
     assert.ok(fired, 'should have fired removed event on rect1');
-    //assert.deepEqual(targets, [rect2, rect1, rect3], 'should contain removed objects');
+    assert.deepEqual(targets, [rect2, rect1, rect3], 'should contain removed objects');
   });
 
   QUnit.test('size', function(assert) {
@@ -573,6 +574,7 @@
       group = new fabric.Group(),
       control = [],
       fired = [],
+      firedOnGroup = [],
       firingControl = [];
 
     group.add(rect1, rect2);
@@ -585,6 +587,7 @@
       assert.deepEqual(group.getObjects(), control, 'should equal control array ' + description);
       assert.deepEqual(fired.map(o => o.id), firingControl.map(o => o.id), 'fired events should equal control array ' + description);
       assert.deepEqual(fired, firingControl, 'fired events should equal control array ' + description);
+      assert.deepEqual(firedOnGroup, firingControl, 'fired events should equal control array ' + description);
     }
 
     assert.ok(typeof group._onObjectAdded === 'function', 'has a standard _onObjectAdded method');
@@ -594,6 +597,9 @@
         fired.push(obj);
       });
     });
+    group.on('object:added', (e) => {
+      firedOnGroup.push(e.target);
+    });
 
     group.insertAt(rect3, 1);
     control.splice(1, 0, rect3);
@@ -738,12 +744,22 @@
   });
 
   QUnit.test('group remove', function(assert) {
-    var rect1 = new fabric.Rect({ top: 1, left: 1, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
-        rect2 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
-        group = new fabric.Group([rect1, rect2]);
+    var rect1 = new fabric.Rect({ top: 1, left: 1, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false }),
+        rect2 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false }),
+        group = new fabric.Group([rect1, rect2]),
+        fired = [];
 
     var coords = group.oCoords;
+    group.on('object:removed', (e) => {
+      assert.equal(e.target, rect2);
+      fired.push(group);
+    });
+    rect2.on('removed', (e) => {
+      assert.equal(e.target, group);
+      fired.push(rect2);
+    });
     group.remove(rect2);
+    assert.deepEqual(fired, [group, rect2], 'should fire removed in correct order');
     var newCoords = group.oCoords;
     assert.notEqual(coords, newCoords, 'object coords have been recalculated - remove');
   });

test/unit/object.js

@@ -818,6 +818,21 @@
     assert.ok(object.isDescendantOf(object) === false);
   });
 
+  QUnit.test('getParent', function (assert) {
+    var object = new fabric.Object();
+    var parent = new fabric.Object();
+    var activeSelection = new fabric.Object({ type: 'activeSelection' });
+    assert.ok(typeof object.getParent === 'function');
+    assert.equal(object.getParent(), undefined);
+    object.group = parent;
+    object.canvas = canvas;
+    assert.equal(object.getParent(), parent);
+    parent.canvas = canvas;
+    assert.equal(parent.getParent(), canvas);
+    parent.group = activeSelection;
+    assert.equal(parent.getParent(), canvas, 'should not consider active selection as parent');
+  });
+
   QUnit.test('getAncestors', function (assert) {
     var object = new fabric.Object();
     var parent = new fabric.Object();