diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index a44d2552..014bec88 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -13,7 +13,6 @@ */ _shouldGroup: function(e, target) { var activeObject = this._activeObject; - return activeObject && this._isSelectionKeyPressed(e) && target && target.selectable && this.selection && (activeObject !== target || activeObject.type === 'activeSelection'); }, @@ -32,7 +31,7 @@ // if it's a group, find target again, using activeGroup objects target = this.findTarget(e, true); // if even object is not found or we are on activeObjectCorner, bail out - if (!target) { + if (!target || !target.selectable) { return; } } diff --git a/test/unit/canvas_events.js b/test/unit/canvas_events.js index e59b4183..8740d6b1 100644 --- a/test/unit/canvas_events.js +++ b/test/unit/canvas_events.js @@ -6,6 +6,7 @@ QUnit.module('fabric.Canvas events mixin', { beforeEach: function() { canvas.cancelRequestedRender(); + canvas.viewportTransform = [1, 0, 0, 1, 0, 0]; upperCanvasEl.style.display = ''; canvas.controlsAboveOverlay = fabric.Canvas.prototype.controlsAboveOverlay; canvas.preserveObjectStacking = fabric.Canvas.prototype.preserveObjectStacking; @@ -171,6 +172,66 @@ canvas.__onMouseUp(e); }); + QUnit.test('specific bug #5317 for shift+click and active selection', function(assert) { + var greenRect = new fabric.Rect({ + width: 300, + height: 300, + left: 0, + top: 0, + fill: 'green', + selectable: false + }); + canvas.add(greenRect); + + // add green, half-transparent circle + var redCircle = new fabric.Circle({ + radius: 40, + left: 200, + top: 100, + fill: 'red', + opacity: 0.5 + }); + canvas.add(redCircle); + + // add green, half-transparent circle + var blueCircle = new fabric.Circle({ + radius: 40, + left: 0, + top: 0, + fill: 'blue', + opacity: 0.5 + }); + canvas.add(blueCircle); + var e = { clientX: 40, clientY: 40, which: 1, target: canvas.upperCanvasEl }; + canvas.__onMouseDown(e); + assert.equal(canvas._activeObject, blueCircle, 'blue circle is selected with first click'); + canvas.__onMouseUp(e); + var e2 = { clientX: 240, clientY: 140, which: 1, target: canvas.upperCanvasEl, shiftKey: true }; + canvas.__onMouseDown(e2); + var selection = canvas.getActiveObjects(); + assert.equal(selection[1], blueCircle, 'blue circle is still selected'); + assert.equal(selection[0], redCircle, 'red circle is selected with shift click'); + canvas.__onMouseUp(e2); + var e3 = { clientX: 140, clientY: 90, which: 1, target: canvas.upperCanvasEl, shiftKey: true }; + canvas.__onMouseDown(e3); + var selection = canvas.getActiveObjects(); + canvas.on('mouse:down', function(options) { + assert.equal(options.target, greenRect, 'green rectangle was the target'); + }); + assert.equal(selection[1], blueCircle, 'blue circle is still selected 2'); + assert.equal(selection[0], redCircle, 'red circle is still selected 2'); + assert.equal(selection.length, 2, 'no other object have been selected'); + canvas.__onMouseUp(e3); + var e4 = { clientX: 290, clientY: 290, which: 1, target: canvas.upperCanvasEl }; + canvas.__onMouseDown(e4); + var selection = canvas.getActiveObjects(); + canvas.on('mouse:down', function(options) { + assert.equal(options.target, greenRect, 'green rectangle was the target 2'); + }); + assert.equal(selection.length, 0, 'no other object have been selected because green rect is unselectable'); + canvas.__onMouseUp(e4); + }); + QUnit.test('mouse:up isClick = true', function(assert) { var e = { clientX: 30, clientY: 30, which: 1, target: canvas.upperCanvasEl }; var isClick = false; @@ -456,7 +517,6 @@ fabric.Canvas.prototype._onResize = originalResize; }); - QUnit.test('actionIsDisabled ', function(assert) { assert.ok(typeof fabric.Canvas.prototype.actionIsDisabled === 'function', 'actionIsDisabled is a function'); var key = canvas.altActionKey;