feat: Parallel animation added #25
Conversation
tomek-r
requested review from
bchelkowski and
adamczopek
and removed request for
a team
| ' @param {Object} [options={}] | ||
| ' @param {Float} options.delay | ||
| ' @param {Boolean} options.repeat | ||
| ' @param {AnimatorFactory~Options} options.animations |
There was a problem hiding this comment.
AA of {AnimatorFactory~Options}?
According to this, it could be something like:
@param {Object.<string, AnimatorFactory~Options>}
There was a problem hiding this comment.
AnimatorFactory~Options is already defined in AnimatoFactory.brs so it's ok, isn't it?
Anyway, in my opinion, instead of unclear AA (as Radek mentioned, docs are incorrect now, because AnimatorFactory~Options is a type of value of AA) I'd switch to an array of objects like { elementId, animatorOptions }
There was a problem hiding this comment.
AnimatorFactory~Options is already defined in AnimatoFactory.brs so it's ok, isn't it?
true, missed that when updating comment 😅 will update my previous comment then
| ' @param {AnimatorFactory~Options} options.animations | ||
| ' @param {Node} element | ||
| ' @returns {Object} - ParallelAnimation component | ||
| prototype.createAnimation = function(name as String, options = {} as Object, element = invalid as Object) as Object |
| for each animationOption in options.animations.items() | ||
| parallelAnimationNode.appendChild(factory.createAnimation(animationOption.key, animationOption.value)) | ||
| end for |
There was a problem hiding this comment.
AnimatorFactory.createAnimation has such an interface: function (name as String, options = {} as Object, element = Invalid as Object)
So if you implement my suggestion (https://github.com/getndazn/kopytko-utils/pull/25/files#r1378563587) you could iterate through options.animations objects and code would look clearer:
| for each animationOption in options.animations.items() | |
| parallelAnimationNode.appendChild(factory.createAnimation(animationOption.key, animationOption.value)) | |
| end for | |
| for each animation in options.animations | |
| parallelAnimationNode.appendChild(factory.createAnimation(animation.elementId, animation.animatorOptions)) | |
| end for |
There was a problem hiding this comment.
From the definition point of view clearer is:
{
someAnimationConfig: {
animations: {
gradient: {
duration:0.2
easeFunction: "linear"
fields: [...]
}
}
}
}
vs
{
someAnimationConfig: {
animations: [
{
name: "gradient",
options: {
duration: 0.2,
easeFunction: "linear",
fields: [],
}
}
]
}
}
Less code and more compact imho.
| ' @import /components/KopytkoTestSuite.brs from @dazn/kopytko-unit-testing-framework | ||
| ' @import /components/rokuComponents/Animation.brs | ||
| ' @mock /components/animator/AnimatorFactory.brs | ||
| function TestSuite__ParallelAnimationFactory() as Object |
There was a problem hiding this comment.
A unit test testing multiple elements is missing
| delay: Csng(0.2), | ||
| duration: Csng(20), | ||
| easeFunction: "linear", | ||
| easeInPercent: 0.1, | ||
| easeOutPercent: 0.2, | ||
| optional: false, | ||
| repeat: false, |
There was a problem hiding this comment.
I don't understand, aren't fields necessary? Otherwise, it makes no sense to me
src/components/animation/_tests/ParallelAnimationFactory.test.brs
Outdated
Show resolved
Hide resolved
Co-authored-by: Błażej Chełkowski <bchelkow@gmail.com>
# [2.5.0](v2.4.2...v2.5.0) (2023-11-08) ### Features * Parallel animation added ([#25](#25)) ([dc9d32f](dc9d32f))
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you implement:
ParallelAnimation
How did you implement it:
Added new Parallel Animation wrapper.
How can we verify it:
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO