Move source of truth for duration and repeat style to the execution phase#49
Conversation
| /// total duration equal to the duration of one cycle (the animation's `duration`) multiplied by the number of | ||
| /// cycles (as specified by the animation's `repeatStyle`). | ||
| /// total duration equal to the duration of one cycle (the animation's `implicitDuration`) multiplied by the number | ||
| /// of cycles (as specified by the animation's `implicitRepeatStyle`). |
There was a problem hiding this comment.
I want to follow this up with an effort to be more consistent with the terminology we use in the project - both in the long form documentation and headerdocs, as well as some of the (private) variable names. For example, we talk about the "total" duration here, but I don't think that's the most descriptive term. When talking about durations, I've started differentiating by saying:
- The time from the start of the execution phase to when the animation completes is the
end-to-end duration. This is usually a calculated value based on the cycle duration and the repeat style (when available). - The time for the animation to go from a relative timestamp of 0 to 1 (or 1 to 0) is the
cycle duration. This is typically how consumers specify the duration. - The time for the animation to go between two arbitrary relative timestamps is the
segment duration. This is used in a few places like snapshot testing and the upcoming interactive animations.
There was a problem hiding this comment.
This makes sense, total can be ambiguous when animation has multiple phases. end-to-end sounds good but feels a bit clumsy as variable name, what about overall duration?
There was a problem hiding this comment.
Yeah, I could see "overall" working for that. I filed to #52 to track updating the documentation.
37c50ae to
b995d5d
Compare
|
Pulled the fixes to the pod lint command into a separate PR (#51) |
|
This was commented on the wrong PR 🤦 Moved it to #35 (comment). |
…hase * Adds `duration` and `repeatStyle` parameters to the `Animation.perform(...)` and `AnimationQueue.enqueue(...)` methods to allow for specifying an explicit duration and repeat style at the start of the execution phase. * Renames the `duration` properties on `Animation` and `AnimationGroup` to `implicitDuration`. * Renames the `repeatStyle` properties on `Animation` and `AnimationGroup` to `implicitRepeatStyle`. * Renames `AnimationRepeatStyle.none` to `.noRepeat` to avoid an ambiguous reference when using an optional repeat style. * Updates a lot of headerdocs to better explain how the duration and repeat styles are resolved.
b995d5d to
537422d
Compare
seanho
left a comment
There was a problem hiding this comment.
Good stuff. The implicit naming adds clarify and easy to reason about.
| /// total duration equal to the duration of one cycle (the animation's `duration`) multiplied by the number of | ||
| /// cycles (as specified by the animation's `repeatStyle`). | ||
| /// total duration equal to the duration of one cycle (the animation's `implicitDuration`) multiplied by the number | ||
| /// of cycles (as specified by the animation's `implicitRepeatStyle`). |
There was a problem hiding this comment.
This makes sense, total can be ambiguous when animation has multiple phases. end-to-end sounds good but feels a bit clumsy as variable name, what about overall duration?
durationandrepeatStyleparameters to theAnimation.perform(...)andAnimationQueue.enqueue(...)methods to allow for specifying an explicit duration and repeat style at the start of the execution phase.durationproperties onAnimationandAnimationGrouptoimplicitDuration.repeatStyleproperties onAnimationandAnimationGrouptoimplicitRepeatStyle.AnimationRepeatStyle.noneto.noRepeatto avoid an ambiguous reference when using an optional repeat style.Resolves #43.