Fix the draggability of small objects (#5379)

* change to action logic

* fixed test

* added some tests

package.json

@@ -77,5 +77,6 @@
   "engines": {
     "node": ">=4.0.0"
   },
-  "main": "./dist/fabric.js"
+  "main": "./dist/fabric.js",
+  "dependencies": {}
 }

src/canvas.class.js

@@ -644,8 +644,8 @@
     /**
      * @private
      */
-    _getActionFromCorner: function(target, corner, e) {
-      if (!corner) {
+    _getActionFromCorner: function(alreadySelected, corner, e) {
+      if (!corner || !alreadySelected) {
         return 'drag';
       }
 
@@ -668,14 +668,14 @@
      * @param {Event} e Event object
      * @param {fabric.Object} target
      */
-    _setupCurrentTransform: function (e, target) {
+    _setupCurrentTransform: function (e, target, alreadySelected) {
       if (!target) {
         return;
       }
 
       var pointer = this.getPointer(e),
           corner = target._findTargetCorner(this.getPointer(e, true)),
-          action = this._getActionFromCorner(target, corner, e),
+          action = this._getActionFromCorner(alreadySelected, corner, e),
           origin = this._getOriginFromCorner(target, corner);
 
       this._currentTransform = {
@@ -1148,8 +1148,11 @@
     /**
      * Method that determines what object we are clicking on
      * the skipGroup parameter is for internal use, is needed for shift+click action
+     * 11/09/2018 TODO: would be cool if findTarget could discern between being a full target
+     * or the outside part of the corner.
      * @param {Event} e mouse event
      * @param {Boolean} skipGroup when true, activeGroup is skipped and only objects are traversed through
+     * @return {fabric.Object} the target found
      */
     findTarget: function (e, skipGroup) {
       if (this.skipTargetFind) {

src/mixins/canvas_events.mixin.js

@@ -611,11 +611,12 @@
       }
 
       if (target) {
+        var alreadySelected = target === this._activeObject;
         if (target.selectable) {
           this.setActiveObject(target, e);
         }
         if (target === this._activeObject && (target.__corner || !shouldGroup)) {
-          this._setupCurrentTransform(e, target);
+          this._setupCurrentTransform(e, target, alreadySelected);
         }
       }
       this._handleEvent(e, 'down');

test/unit/canvas.js

@@ -2171,7 +2171,16 @@
       clientY: canvasOffset.top + rect.oCoords.tl.corner.tl.y + 1,
       target: rect
     };
-    canvas._setupCurrentTransform(eventStub, rect);
+
+    canvas._setupCurrentTransform(eventStub, rect, false);
+    t = canvas._currentTransform;
+    assert.equal(t.target, rect, 'should have rect as a target');
+    assert.equal(t.action, 'drag', 'should setup drag since the object was not selected');
+    assert.equal(t.corner, 'tl', 'tl selected');
+    assert.equal(t.shiftKey, undefined, 'shift was not pressed');
+
+    var alreadySelected = true;
+    canvas._setupCurrentTransform(eventStub, rect, alreadySelected);
     t = canvas._currentTransform;
     assert.equal(t.target, rect, 'should have rect as a target');
     assert.equal(t.action, 'scale', 'should target a corner and setup scale');
@@ -2186,7 +2195,7 @@
       target: rect,
       shiftKey: true
     };
-    canvas._setupCurrentTransform(eventStub, rect);
+    canvas._setupCurrentTransform(eventStub, rect, alreadySelected);
     t = canvas._currentTransform;
     assert.equal(t.target, rect, 'should have rect as a target');
     assert.equal(t.action, 'skewY', 'should target a corner and setup skew');
@@ -2199,7 +2208,7 @@
       clientY: canvasOffset.top + rect.oCoords.mtr.y,
       target: rect,
     };
-    canvas._setupCurrentTransform(eventStub, rect);
+    canvas._setupCurrentTransform(eventStub, rect, alreadySelected);
     t = canvas._currentTransform;
     assert.equal(t.target, rect, 'should have rect as a target');
     assert.equal(t.action, 'rotate', 'should target a corner and setup rotate');

test/unit/canvas_events.js

@@ -298,6 +298,39 @@
     assert.equal(count2, 1, 'object:moved fired');
   });
 
+  QUnit.test('drag small object when mousemove + drag, not active', function(assert) {
+    var e = { clientX: 2, clientY: 2, which: 1 };
+    var e1 = { clientX: 4, clientY: 4, which: 1 };
+    var e2 = { clientX: 6, clientY: 6, which: 1 };
+    var rect = new fabric.Rect({ left: 0, top: 0, width: 3, height: 3, strokeWidth: 0 });
+    canvas.add(rect);
+    canvas.__onMouseDown(e);
+    canvas.__onMouseMove(e1);
+    canvas.__onMouseMove(e2);
+    canvas.__onMouseUp(e2);
+    assert.equal(rect.top, 4, 'rect moved by 4 pixels top');
+    assert.equal(rect.left, 4, 'rect moved by 4 pixels left');
+    assert.equal(rect.scaleX, 1, 'rect did not scale Y');
+    assert.equal(rect.scaleY, 1, 'rect did not scale X');
+  });
+
+  QUnit.test('scale small object when mousemove + drag, active', function(assert) {
+    var e = { clientX: 2, clientY: 2, which: 1 };
+    var e1 = { clientX: -4, clientY: -4, which: 1 };
+    var e2 = { clientX: -6, clientY: -6, which: 1 };
+    var rect = new fabric.Rect({ left: 0, top: 0, width: 3, height: 3, strokeWidth: 0 });
+    assert.equal(rect.scaleX, 1, 'rect not scaled X');
+    assert.equal(rect.scaleY, 1, 'rect not scaled Y');
+    canvas.add(rect);
+    canvas.setActiveObject(rect);
+    canvas.__onMouseDown(e);
+    canvas.__onMouseMove(e1);
+    canvas.__onMouseMove(e2);
+    canvas.__onMouseUp(e2);
+    assert.equal(rect.scaleX, 3, 'rect scaled X');
+    assert.equal(rect.scaleY, 3, 'rect scaled Y');
+  });
+
   QUnit.test('avoid multiple bindings', function(assert) {
     var c = new fabric.Canvas();
     var eventsArray = [