Make generic _set aware of real change property (#4415)

* first change * added a small test
fabricjs · Oct 26, 2017 · 141f60a · 141f60a
1 parent c678704
commit 141f60a
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 3 deletions.
diff --git a/src/shapes/object.class.js b/src/shapes/object.class.js
@@ -884,7 +884,8 @@
      * @return {fabric.Object} thisArg
      */
     _set: function(key, value) {
-      var shouldConstrainValue = (key === 'scaleX' || key === 'scaleY');
+      var shouldConstrainValue = (key === 'scaleX' || key === 'scaleY'),
+          isChanged = this[key] !== value;
 
       if (shouldConstrainValue) {
         value = this._constrainScale(value);
@@ -906,14 +907,14 @@
 
       this[key] = value;
 
-      if (this.cacheProperties.indexOf(key) > -1) {
+      if (isChanged && this.cacheProperties.indexOf(key) > -1) {
         if (this.group) {
           this.group.set('dirty', true);
         }
         this.dirty = true;
       }
 
-      if (this.group && this.stateProperties.indexOf(key) > -1 && this.group.isOnACache()) {
+      if (isChanged && this.group && this.stateProperties.indexOf(key) > -1 && this.group.isOnACache()) {
         this.group.set('dirty', true);
       }
 

diff --git a/test/unit/object.js b/test/unit/object.js
@@ -1225,4 +1225,22 @@
     object.shadow.offsetX = 1;
     assert.equal(object.willDrawShadow(), true, 'object will drawShadow');
   });
+
+  QUnit.test('_set  change a property', function(assert) {
+    var object = new fabric.Object({ fill: 'blue' });
+    object._set('fill', 'red');
+    assert.equal(object.fill, 'red', 'property changed');
+  });
+  QUnit.test('_set can rise the dirty flag', function(assert) {
+    var object = new fabric.Object({ fill: 'blue' });
+    object.dirty = false;
+    object._set('fill', 'red');
+    assert.equal(object.dirty, true, 'dirty is rised');
+  });
+  QUnit.test('_set rise dirty flag only if value changed', function(assert) {
+    var object = new fabric.Object({ fill: 'blue' });
+    object.dirty = false;
+    object._set('fill', 'blue');
+    assert.equal(object.dirty, false, 'dirty is not rised');
+  });
 })();