On shift select and multiselection, check for onSelect callback (#5348)

* add onSelect on the grouping logic * added test for group selector * another test
fabricjs · Oct 28, 2018 · 8115d69 · 8115d69
1 parent 847fe90
commit 8115d69
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 4 deletions.
diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js
@@ -14,7 +14,7 @@
     _shouldGroup: function(e, target) {
       var activeObject = this._activeObject;
       return activeObject && this._isSelectionKeyPressed(e) && target && target.selectable && this.selection &&
-            (activeObject !== target || activeObject.type === 'activeSelection');
+            (activeObject !== target || activeObject.type === 'activeSelection') && !target.onSelect({ e: e });
     },
 
     /**
@@ -24,6 +24,7 @@
      */
     _handleGrouping: function (e, target) {
       var activeObject = this._activeObject;
+      // avoid multi select when shift click on a corner
       if (activeObject.__corner) {
         return;
       }
@@ -96,7 +97,7 @@
      */
     _groupSelectedObjects: function (e) {
 
-      var group = this._collectObjects(),
+      var group = this._collectObjects(e),
           aGroup;
 
       // do not create group for 1 element only
@@ -114,7 +115,7 @@
     /**
      * @private
      */
-    _collectObjects: function() {
+    _collectObjects: function(e) {
       var group = [],
           currentObject,
           x1 = this._groupSelector.ex,
@@ -129,7 +130,7 @@
       for (var i = this._objects.length; i--; ) {
         currentObject = this._objects[i];
 
-        if (!currentObject || !currentObject.selectable || !currentObject.visible) {
+        if (!currentObject || !currentObject.selectable || !currentObject.visible || currentObject.onSelect({ e: e })) {
           continue;
         }
 

diff --git a/test/unit/canvas.js b/test/unit/canvas.js
@@ -573,6 +573,67 @@
     canvas.selectionFullyContained = false;
   });
 
+  QUnit.test('_collectObjects does not collect objects that have onSelect returning true', function(assert) {
+    canvas.selectionFullyContained = false;
+    var rect1 = new fabric.Rect({ width: 10, height: 10, top: 2, left: 2 });
+    rect1.onSelect = function() {
+      return true;
+    };
+    var rect2 = new fabric.Rect({ width: 10, height: 10, top: 2, left: 2 });
+    canvas.add(rect1, rect2);
+    canvas._groupSelector = {
+      top: 20,
+      left: 20,
+      ex: 1,
+      ey: 1
+    };
+    var collected = canvas._collectObjects();
+    assert.equal(collected.length, 1, 'objects are in the same position buy only one gets selected');
+    assert.equal(collected[0], rect2, 'contains rect2 but not rect 1');
+  });
+
+  QUnit.test('_shouldGroup return false if onSelect return true', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return true;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = true;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, false, 'if onSelect returns true, shouldGroup return false');
+  });
+
+  QUnit.test('_shouldGroup return true if onSelect return false and selectionKey is true', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return false;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = true;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, true, 'if onSelect returns false, shouldGroup return true');
+  });
+
+  QUnit.test('_shouldGroup return false if selectionKey is false', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return false;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = false;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, false, 'shouldGroup return false');
+  });
+
   QUnit.test('_fireSelectionEvents fires multiple things', function(assert) {
     var rect1Deselected = false;
     var rect3Selected = false;