Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single API for animating colors and other properties #6191

Merged
merged 5 commits into from
Mar 4, 2020
Merged

Single API for animating colors and other properties #6191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd900b2
animateColor from Object.animate
yassilah Mar 2, 2020
9dc11ca
animateColor from Object.animate
yassilah Mar 2, 2020
664152f
Remove borderColor from colorProperties
yassilah Mar 2, 2020
cdd5a82
Rename "propIsText" to "propIsColor" + es5 support
yassilah Mar 2, 2020
3064455
Remove dist
yassilah Mar 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions src/mixins/animation.mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
propPair = property.split('.');
}

var propIsColor = _this.colorProperties.indexOf(property) > -1 || (propPair && _this.colorProperties.indexOf(propPair[1]) > -1);

var currentValue = propPair
? this.get(propPair[0])[propPair[1]]
: this.get(property);
Expand All @@ -182,23 +184,25 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
options.from = currentValue;
}

if (~to.indexOf('=')) {
to = currentValue + parseFloat(to.replace('=', ''));
}
else {
to = parseFloat(to);
if (!propIsColor) {
if (~to.indexOf('=')) {
to = currentValue + parseFloat(to.replace('=', ''));
}
else {
to = parseFloat(to);
}
}

fabric.util.animate({
var _options = {
startValue: options.from,
endValue: to,
byValue: options.by,
easing: options.easing,
duration: options.duration,
abort: options.abort && function() {
abort: options.abort && function () {
return options.abort.call(_this);
},
onChange: function(value, valueProgress, timeProgress) {
onChange: function (value, valueProgress, timeProgress) {
if (propPair) {
_this[propPair[0]][propPair[1]] = value;
}
Expand All @@ -210,14 +214,21 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
}
options.onChange && options.onChange(value, valueProgress, timeProgress);
},
onComplete: function(value, valueProgress, timeProgress) {
onComplete: function (value, valueProgress, timeProgress) {
if (skipCallbacks) {
return;
}

_this.setCoords();
options.onComplete && options.onComplete(value, valueProgress, timeProgress);
}
});
};

if (propIsColor) {
fabric.util.animateColor(_options.startValue, _options.endValue, _options.duration, _options);
}
else {
fabric.util.animate(_options);
}
}
});
8 changes: 8 additions & 0 deletions src/shapes/object.class.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,14 @@
' strokeLineCap strokeDashOffset strokeLineJoin strokeMiterLimit backgroundColor clipPath'
).split(' '),

/**
* List of properties to consider for animating colors.
* @type Array
*/
colorProperties: (
'fill stroke backgroundColor'
).split(' '),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is borderColor something we should consider animatable? if yes, why not also cornerColor, cornerStrokeColor and all the rest?
I would leave borderColor out for now. But of course if you think is really important, argument why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i personally wouldn't use either, it was just for testing so I think we can leave it out for now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i want to merge this as it is, but i think i will come back and move colorProperties to animation util level maybe.
Too busy to think of it now and not so important.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the reason I put it on the object class is because I thought it might be easier to create a new shape class with a custom property name that involved coloring but as you like :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know but so many options.


/**
* a fabricObject that, without stroke define a clipping area with their shape. filled in black
* the clipPath object gets used when the object has rendered, and the context is placed in the center
Expand Down
18 changes: 18 additions & 0 deletions test/unit/animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@
}, 1000);
});

QUnit.test('animate with color', function(assert) {
var done = assert.async(), properties = fabric.Object.prototype.colorProperties,
object = new fabric.Object();

properties.forEach(function (prop, index) {
object.set(prop, 'red');
object.animate(prop, 'blue');
assert.ok(true, 'animate without options does not crash');

setTimeout(function () {
assert.equal(object[prop], new fabric.Color('blue').toRgba(), 'property [' + prop + '] has been animated');
if (index === properties.length - 1) {
done();
}
}, 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use the animation callback instead of a timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, i was just copying the other tests but i can do that! you mean in a onComplete callback?

});
});

QUnit.test('animate with decrement', function(assert) {
var done = assert.async();
var object = new fabric.Object({ left: 20, top: 30, width: 40, height: 50, angle: 43 });
Expand Down