w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.44k stars 657 forks source link

[scroll-animations-1][web-animations-2] getCurrentTime is self-inconsistent wrt representing time #8765

Open fantasai opened 1 year ago

fantasai commented 1 year ago

In order to represent time in relation to named timeline ranges, we have two options:

If we consider its default behavior as returning the time with respect to a timeline range that encompasses the entire timeline, the current definition for getCurrentTime() switches between the two modes depending on whether you specify a range name or not. (!)

This doesn't feel unnatural to scroll timelines, because their “absolute timeline units” are percentages of the timeline (which itself has a finite range)... We would probably have noticed the discrepency sooner if they used length-based time. :/

But it runs into problems if we extend the named timeline concept to other types of timelines, such as time-based timelines. These bring in (potentially) two differences:

I think we might want to rethink this API. Some possible options:

I'm not sure which direction we want to go... @bramus, thoughts?

bramus commented 1 year ago

I can see that as an author you’d sometimes want to know the relative “time” (i.e. a percentage) and sometimes you’d want the absolute “time”. We could go with two different methods getRelative… and getAbsolute… for this, or add an extra argument to the property bag of the existing one. Undecided that the default argument value – relative or absolute – should be in the latter case.

Side note: seeing that I put time between quotes in the previous paragraph and @fantasai used the word “progress” – and I also hinted at it here – maybe it’s better to rename the method to getProgress()?

fantasai commented 1 year ago

@bramus ... so maybe we want parallel getCurrentTime and getCurrentProgress functions where the former is in .currentTime units and the latter is in percentages (always) and returns null on non-finite ranges?

bramus commented 1 year ago

/ping @flackr to get your take on this. I think this needs to be tackled before shipping.

birtles commented 1 year ago

I agree this should be resolved before shipping. I wonder how important the API even is? Do authors need it? Is it needed for WPT? If we're not sure what the ergonomics/naming/use cases are for this, would it make sense to ship without it?

flackr commented 1 year ago

I agree with @birtles that I think this is purely additive to scroll driven animations and doesn't need to be part of the initial API. I'm in favor of shipping without it and adding it later.

flackr commented 1 year ago

The use cases I've seen presented for this have been about matching an animation's progress at which point it seems like we should provide an animation-wide progress or suggest developers access the effect progress rather than trying to make an API which usually aligns with an animation effect.

That said, if we think an API is needed on the timeline for use cases which aren't trying to match an animation progress, I would suggest using terminology like progress if we are going to return progress through a range rather than an absolute time.

fantasai commented 1 year ago

So we resolved to add .progress to the Animation API in https://github.com/w3c/csswg-drafts/issues/8799 ; but that still leaves this issue open. If we're deferring getCurrentTime() from Level 1, we need to remove it from the specs. If we want to fix this issue, then we need to pick a solution.

birtles commented 1 year ago

So long as we don't have a strong use case (it seems like .progress covers the use cases we're aware of) and we don't have a clear picture of what the shape of the API should me, I'm very much in favor of dropping it from Level 1.

css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [scroll-animations-1][web-animations-2] getCurrentTime is self-inconsistent wrt representing time, and agreed to the following:

The full IRC log of that discussion <dael> flackr: We've had a bunch of debate about best way getCurrentTime on a timeline should work, if needed, what are use cases. Resolve dto add progress to animation to handle common use case
<dael> flackr: With that, think we should remove getCurrentTime from the APIa nd re-add when we have clearer sense of use cases
<dael> fantasai: I'm fine with deferring. It means there is no real way to figure out where you are in a timeline range. So you don't know when unless you make an animation and measure progress. We don't have API to say how ranges relate to rest of timeline
<dael> fantasai: But I'm fine with deferring. Not a problem
<dael> Rossen_: Anyone else?
<dael> Rossen_: Objections to defer on getCurrentTime
<dael> RESOLVED: Defer on getCurrentTime for L1
<dael> Rossen_: I'm assuming bramus is on same page as you? I don't see us doing a disservice for any commentor