Use the new mousedown:before to understand if an object is already se…

…lected on mousedown (#5010) * fix something * other changes
fabricjs · May 30, 2018 · 6db1b1d · 6db1b1d
1 parent b77957f
commit 6db1b1d
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 28 deletions.
diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js
@@ -161,7 +161,6 @@
       this.setCursor(this.defaultCursor);
       // clear selection and current transformation
       this._groupSelector = null;
-      this._currentTransform = null;
     }
   });
 

diff --git a/src/mixins/itext_behavior.mixin.js b/src/mixins/itext_behavior.mixin.js
@@ -15,10 +15,9 @@
       this.mouseMoveHandler = this.mouseMoveHandler.bind(this);
     },
 
-    onDeselect: function(options) {
+    onDeselect: function() {
       this.isEditing && this.exitEditing();
       this.selected = false;
-      fabric.Object.prototype.onDeselect.call(this, options);
     },
 
     /**
@@ -59,13 +58,13 @@
      * @private
      */
     _initCanvasHandlers: function(canvas) {
-      canvas._mouseUpITextHandler = (function() {
+      canvas._mouseUpITextHandler = function() {
         if (canvas._iTextInstances) {
           canvas._iTextInstances.forEach(function(obj) {
             obj.__isMousedown = false;
           });
         }
-      }).bind(this);
+      };
       canvas.on('mouse:up', canvas._mouseUpITextHandler);
     },
 

diff --git a/src/mixins/itext_click_behavior.mixin.js b/src/mixins/itext_click_behavior.mixin.js
@@ -12,7 +12,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
 
     this.__lastPointer = { };
 
-    this.on('mousedown', this.onMouseDown.bind(this));
+    this.on('mousedown', this.onMouseDown);
   },
 
   /**
@@ -24,7 +24,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
       return;
     }
     this.__newClickTime = +new Date();
-    var newPointer = this.canvas.getPointer(options.e);
+    var newPointer = options.pointer;
     if (this.isTripleClick(newPointer)) {
       this.fire('tripleclick', options);
       this._stopEvent(options.e);
@@ -82,10 +82,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
     if (!this.canvas || !this.editable || (options.e.button && options.e.button !== 1)) {
       return;
     }
-    var pointer = this.canvas.getPointer(options.e);
 
-    this.__mousedownX = pointer.x;
-    this.__mousedownY = pointer.y;
     this.__isMousedown = true;
 
     if (this.selected) {
@@ -102,21 +99,25 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
   },
 
   /**
-   * Initializes "mousedown" event handler
+   * Default event handler for the basic functionalities needed on mousedown:before
+   * can be overridden to do something different.
+   * Scope of this implementation is: verify the object is already selected when mousing down
    */
-  initMousedownHandler: function() {
-    this.on('mousedown', this._mouseDownHandler);
+  _mouseDownHandlerBefore: function(options) {
+    if (!this.canvas || !this.editable || (options.e.button && options.e.button !== 1)) {
+      return;
+    }
+    if (this === this.canvas._activeObject) {
+      this.selected = true;
+    }
   },
 
   /**
-   * detect if object moved
-   * @private
+   * Initializes "mousedown" event handler
    */
-  _isObjectMoved: function(e) {
-    var pointer = this.canvas.getPointer(e);
-
-    return this.__mousedownX !== pointer.x ||
-           this.__mousedownY !== pointer.y;
+  initMousedownHandler: function() {
+    this.on('mousedown', this._mouseDownHandler);
+    this.on('mousedown:before', this._mouseDownHandlerBefore);
   },
 
   /**
@@ -132,7 +133,9 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
    */
   mouseUpHandler: function(options) {
     this.__isMousedown = false;
-    if (!this.editable || this._isObjectMoved(options.e) || (options.e.button && options.e.button !== 1)) {
+    if (!this.editable ||
+      (options.transform && options.transform.actionPerformed) ||
+      (options.e.button && options.e.button !== 1)) {
       return;
     }
 

diff --git a/src/mixins/object_origin.mixin.js b/src/mixins/object_origin.mixin.js
@@ -250,13 +250,6 @@
     _getLeftTopCoords: function() {
       return this.translateToOriginPoint(this.getCenterPoint(), 'left', 'top');
     },
-
-    /**
-    * Callback; invoked right before object is about to go from active to inactive
-    */
-    onDeselect: function() {
-      /* NOOP */
-    }
   });
 
 })();
diff --git a/test/unit/itext_click_behaviour.js b/test/unit/itext_click_behaviour.js
@@ -27,4 +27,13 @@
     var selection = iText._getNewSelectionStartFromOffset({ y: 1, x: 1000 }, 500, 520, index, jlen);
     assert.equal(selection, index, 'index value was NOT moved to next char, since is already at end of text');
   });
+  QUnit.test('_mouseDownHandlerBefore set up selected property', function(assert) {
+    var iText = new fabric.IText('test need some word\nsecond line');
+    assert.equal(iText.selected, undefined, 'iText has no selected property');
+    iText.canvas = {
+      _activeObject: iText,
+    };
+    iText._mouseDownHandlerBefore({ e: {} });
+    assert.equal(iText.selected, true, 'iText has selected property');
+  });
 })();