-
Notifications
You must be signed in to change notification settings - Fork 688
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.
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
[scroll-animations-1] currentTime for animations attached to a timeline attachment range #8669
Comments
The current time of an animation is spec'd in web-animations-1 4.4.3 as the following:
In this sense, the current time is in the units of the timeline time and is working as expected. I can see how this is confusing when the timeline time is also in percentage units, but it is consistent with how the currentTime in a regular animation does not give you the progress, but rather the timeline time that has "passed" since the start of the animation. |
I agree with @flackr that this is working as expected. Timeline attachment ranges are a property of the animation, not of the timeline. .currentTime is answering "what time is it on this timeline", not "how far has this animation progressed". Note: We decided to go with percentages for measuring "time" on scroll/view timelines, but we could have just as easily gone with pixels; maybe the intuitive answer here would be different if we had gone with pixels? |
I see. Maybe then, for author convenience, we could alter |
@bramus That would require the timeline to belong to an animation, i.e. have a one-to-one mapping between a timeline and an animation, right? Because each animation can have its own animation attachment range. But isn't it the case that a timeline can be associated with multiple animations? |
Good call. Could be solved by pass the I think that would rhyme nicely with the suggested property bag approach, no? const { timeline, rangeStart, rangeEnd } = animation;
const value = timeline.getCurrentTime({
rangeStart,
rangeEnd
}); |
If you have to pass in the specific animation rangeStart and rangeEnd and you want the animation progress, I feel like the computed timing on the effect is a better fit, e.g.: const value = animation.effect.getComputedTiming().progress; I also have concerns with getCurrentTime being expected to produce a progress. Maybe we could consider whether we should expose progress on the animation object directly? e.g. |
Oh, that would be the easiest indeed, and nicely compliment |
I'm not sure mapping The returned value is based on the various timing properties of the of rootmost effect so it feels like a layering violation to make it look like a property of the animation. Admittedly, we do something similar when we report an animation as "finished" since the point in time at which it finishes is based on the timing of its rootmost effect, if any but we've very deliberately chosen to make that compromise for that specific case and we've gone all in on it, adding I'm not sure if reporting the iteration progress for the rootmost effect deserves that kind of treatment, especially because it is the iteration progress. If we need something to report the fraction from where an animation starts to where it finishes, maybe we should introduce a new concept for that (e.g. "animation progress") and expose that instead? Such a value would presumably be independent of any delays etc. specified on the rootmost effect. I guess it depends on what the ultimate use case is for this? |
I agree, see below.
Yeah I was thinking that if we did introduce something like this it could be the overall animation progress through the active interval. It would of course then have to be undefined for an animation with infinite iterations. Another thing that might have made @bramus's use case simpler is if iteration progress were accessible without needing to call getComputedTiming(), e.g.
I agree we should think about what the particular use case is for this to make sure we're looking at this in the context of how it will be used. If it's for demos such as the one that this issue was filed for I suspect the root effect's iteration progress is what's expected - but then I agree with @birtles that it should be on the root effect and not on the animation. |
In favor of a general approach as described in #8799. |
While playing with ViewTimelines and trying to derive how much of an
animation-range
had already progressed, it surprised me that readingcurrentTime
always returned the progress relative to the entire ViewTimeline range.To demonstrate, I have a demo up on CodePen which you can check using the latest Chrome Canary. The demo tries to resprent the progression within the currently set animation-range for the ViewTimeline and has these controls:
animation-range
On load, the
animation-range
is set toentry 20% entry 80%
.The demo outputs the
currentTime
in three possible ways:Directly read it from the animation.
This doesn’t work when set to a value other than
cover
. This because italways returns the progress measured against the entire range of the ViewTimeline (which coincides withcover
)Read it from the range
Using
getCurrentTime[ForRange]
doesn’t cut it here things here, as it only covers animations that have the same rangeName inanimation-range-start
andanimation-range-end
. Above that, it also only works for when the entire range is covered (from 0% to 100%).Read from the animation’s effect
As a workaround, I turned to reading the animation’s effect, which did the trick:
This, however, felt a lot like jumping through hoops to be honest.
To simplify things for authors, I would like to propose that
currentTime
(the method used in the first approach) returns the progress relative to the timeline attachment range.animation-range
to, for example,entry 50% cover 100%
thecurrentTime
should be relative to that set (custom) range.animation-range
, the progress should be relative to the entire range, which for View Timelines equals tocover
I think this change also lines up with what @fantasai described here (and which was adopted by the WG):
Note that in this case
getCurrentTime[ForRange]
would still be relevant to keep, as it allows you to get the currentTime as if the range was spanning from 0% to 100%.WDYT @fantasai @flackr @kevers-google @birtles?
The text was updated successfully, but these errors were encountered: