[web-animations-2] [scroll-animations-1] Support automatic duration scroll animations #6337
Conversation
kevers-google
changed the title
kevers-google
changed the title
There was a problem hiding this comment.
This PR seems to be including a bunch of unrelated commits, I wonder if you've done an invalid rebase or similar. I think you should be able to rebase your branch onto the remote branch and force push to clean this up.
From @birtles comments on https://github.com/w3c/csswg-drafts/pull/4890/files I think this may still need to rework the intrinsic iteration duration of group effects.
| @@ -910,7 +865,7 @@ The [=current time=] of a {{ScrollTimeline}} is calculated as follows: | |||
|
|
|||
| : If |current scroll offset| is greater than or equal to [=effective | |||
| end offset=]: | |||
| :: The [=current time=] is the [=effective time range=]. | |||
| :: The [=current time=] is the [=duration=]. | |||
|
|
|||
| : Otherwise, | |||
| :: 1. Let |progress| be a result of applying | |||
There was a problem hiding this comment.
Looks like there are still references to effective time range (e.g. on line 876 below). Github won't let me comment on ellided lines.
There was a problem hiding this comment.
Fixed. Can find no remaining references to "time range"
kevers-google
force-pushed
the
scroll-timeline-progress-based
branch
from
d0eaffc
to
b523b28
Compare
|
Rebase should be cleaned up now. |
web-animations-2/Overview.bs
Outdated
| > 1. Compute [=start delay=] to be the result of evaluating | ||
| > <code>|start delay| / |total time| * |timeline duration|</code>. |
There was a problem hiding this comment.
From a spec point of view, I suppose we need different terms to differentiate between the input "start delay" and the calculated "start delay"? Likewise for "iteration duration" and "end delay".
There was a problem hiding this comment.
Added definitions for specified duration and delays, as well as a normalization procedure, while simply copies over the specified values if time based and calls the normalization process if progress based.
web-animations-2/Overview.bs
Outdated
| > 1. Let <var>total time</var> be equal to the result of evaluating | ||
| > <code>|start delay| + |iteration count| * |iteration duration| + | ||
| > |end delay|</code>. |
There was a problem hiding this comment.
I think we should re-use the definition of end time here since it covers a few edge cases like infinite duration with zero iteration count and clamps the result to the positive range.
web-animations-2/Overview.bs
Outdated
| > :: Return 0 | ||
| > | ||
| > : Otherwise | ||
| > :: Return [=timeline duration=] / <a>iteration count</a> |
There was a problem hiding this comment.
This needs to define the result when iteration count is zero.
Also, I think we do want to make this become something like (100% - start - end) / iteration_count as you mentioned in #4890 (comment) in this level of the spec, just not necessarily in this PR.
There was a problem hiding this comment.
done. With note that start and end are presently zero until such time as percentage delays are supported. Plans for a followup CL on CSSNumberish delays.
web-animations-2/Overview.bs
Outdated
| > If the <a lt="timeline duration">duration</dfn> of the Animation's | ||
| > <a>timeline</a> is a percentage, the end delay will be treated as the | ||
| > result of applying the procedure to convert a | ||
| > [=time-based animation to a proportional animation=]. |
There was a problem hiding this comment.
As well as updating the prose, I guess we need to update the type of the delay and endDelay attributes in EffectTiming and OptionalEffectTiming?
There was a problem hiding this comment.
Prefer to defer to a followup PR. The getTiming method returns specified timing, which for now will be time based (or potentially 'auto' for duration). Once we support CSSNumberish delays, then these interfaces will need to be updated. At present the start and end delay are not part of CalculatedTiming. Perhaps they should be since they could differ from the specified values after normalization. Again, prefer to defer.
|
Thank you! |
|
In https://drafts.csswg.org/scroll-animations-1/#the-css-scroll-timeline-rule-interface I still see references to
If I interpret this PR correctly |
|
Good catch bramus. Will have another PR up shortly. |
|
The 'start' and 'end' descriptors are out of date as well since replaced by scrollOffsets. Can clean this up with the remaining time-range reference. |
Change the implicit handling of auto duration animations to use the duration of the supplied timeline if resolved. Scroll-timelines have an implicit timeline duration and the intrinsic iteration duration can be calculated based on this value. Document timelines will have an unresolved timeline duration, which will translate to an intrinsic iteration duration of zero consistent with the currently specified behavior.
If the timeline duration is resolved, effect timing is normalized based on this duration.
This pull request is a replacement for #4890.