[scroll-animations-1] Animation.getCurrentTime is easily confused with Animation.currentTime

[birtles]

I think it's confusing to have both currentTime and getCurrentTime.

Would it be possible to:

1. Move this to the ScrollTimeline interface, (Update: I see now that this is proposed to be used for DocumentTimeline too--I still don't quite follow the use cases but assuming we want to do that, I guess it should stay on AnimationTimeline)
2. Rename it to getCurrentTimeForRange, and
3. Make the rangeName argument non-optional

?

[fantasai]

getCurrentTime is essentially a replacement for .currentTime that returns the same concept, but has more expressivity: So I think it makes sense to keep the same name.

[birtles]

getCurrentTime is essentially a replacement for .currentTime that returns the same concept, but has more expressivity: So I think it makes sense to keep the same name.

I don't think we shouldn't do that. We'll confuse authors unnecessarily. We can't deprecate currentTime and for the overwhelming majority of use cases it's the right choice. We should be encouraging users to continue using currentTime for the sake of compatibility.

For users that need a time for a range, there's getCurrentTimeForRange for which the function name spells out what the string argument represents. (If we really must use getCurrentTime then it needs to take a property bag, not a string.)

[css-meeting-bot]

The CSS Working Group just discussed [scroll-animations-1] Animation.getCurrentTime is easily confused with Animation.currentTime, and agreed to the following:

• RESOLVED: Pending Brian's potential objections, go with `getCurrentTime(rangeName?)`

The full IRC log of that discussion

<fantasai> s/Topic/Subtopic/
<TabAtkins> flackr: Bikeshedding issue
<TabAtkins> flackr: we added an API to get a range-specific current time
<TabAtkins> flackr: But Brian brought up that when you're not specifying a range, and just getting the time of the timeline, there's already an accessor for that
<TabAtkins> flackr: So q is, should this get a range-specific name, or just consider the .currentTime accessor legacy?
<TabAtkins> flackr: Because one bit about the function is we can return the TypedOM object, rather than just the double that .currentTime returns
<TabAtkins> flackr: I'm slightly in preference to this being the new way to get times, so use a generic name, and have range name specified in options with a property bag as Brian suggested
<TabAtkins> astearns: But Brian thinks we shoudl go with getCurrentTimeForRange()?
<TabAtkins> flackr: Yeah, Brian thinks it's confusing to have another API for something we already have an accessor for
<astearns> ack fantasai
<TabAtkins> flackr: I'm not concerned with calling it getCurrenTtimeForRange(), just need to decide
<TabAtkins> fantasai: My thoughts are we'd add this to get the TypedOM object, and to get it relative to a range.
<TabAtkins> fantasai: Feel like taking a property bag is a little awkward from a usage pov
<TabAtkins> fantasai: I think ranges are a fundamental enough concept that having it just be a string argument is fine
<TabAtkins> flackr: His argument for a property bag is that we dont' want to confused authors, and might want to extend in the future
<TabAtkins> fantasai: Those can go in a property bag
<TabAtkins> astearns: I'm a little concerned with resolving without Brian's okay
<TabAtkins> flackr: Least contentious is to resolve on the ForRange naming, we know Brian is okay with that
<TabAtkins> astearns: fantasai, you okay with ForRange?
<TabAtkins> fantasai: Think it depends on if we want a plain getCurrentTime at any point
<TabAtkins> fantasai: Coujld push to apac call and get Brian on
<fantasai> TabAtkins: I'm mildly against extremely long names, especially if the arg is obvious, extra typing is annoying
<fantasai> TabAtkins: not strongly for/against property bag vs positional argument, but lean towards positional b/c fantasai's arg that it's quite fundamental
<fantasai> TabAtkins: if we put in property bag, we can make an overload later
<fantasai> TabAtkins: for API niceness, I think I like what I believe the spec currently specifies
<fantasai> TabAtkins: getCurrentTime("rangename")
<fantasai> TabAtkins: can add property bag later
<fantasai> flackr: Should we resolve this is what we prefer and ask Brian to take a look?
<TabAtkins> astearns: So proposed reoslution is to use the short name with a string argument, reasons above, ping Brian to see if he wants to object
<TabAtkins> astearns: objections?
<TabAtkins> RESOLVED: Pending Brian's potential objections, go with `getCurrentTime(rangeName?)`
<bramus> 8192?

[birtles]

I'm afraid I'm still opposed to the current naming.

I don't think ranges are a fundamental concept. The overwhelming majority usage of the API will continue to be for time-based animations and I think we should bias towards that.

• Adding getCurrentTime makes using the API harder in the common case. When your editor suggests both currentTime and getCurrentTime how do you know which one to choose? You have to look them up to find out what their (subtle) distinction is.
• Generally, authors should prefer using currentTime because it is more backwards compatible.

If we must have getCurrentTime because we expect it to be useful for non-range cases then we really should use a property bag. Doing so is necessary to:

• Make the code readable. How on earth could you guess that the single string argument to getCurrentTime represents a range otherwise?
• Make the API extensible. If we're going with something as generic as "get current time", then we'll no doubt want to add further conditions to that query in future and having an API that switches between positional arguments and property bags is an unnecessary wart.

Now that TS/JS language servers are able to automatically refactor positional arguments into property bags, property bags have become much more common so I believe this approach is also idiomatic.

[bramus]

After playing with the API a bit, I follow what Brian is saying and think getCurrentTimeForRange makes most sense, as that’s what the function essentially does. Other names didn’t cover what the function does.

An alternative name that came up was getProgressForRange but I see how reusing CurrentTime in the name makes sense because currentTime also returns a progress for non-time-based timelines.

Also fine with the property bag approach. To be clear, that would then, for example, be getCurrentTime({ rangeName: "cover" })?

[flackr]

What is the use case for this API? What would a developer do given the "time" of various phases on the timeline?

Given our discussion in #8669 it seems the thing @bramus would like would be best provided by a more convenient way to get a particular animation's progress.

[LeaVerou]

Per the TAG Design Principles, optional and/or primitive arguments should be accepted through dictionaries, not positional arguments.
In this case, this would mean getCurrentTime({ rangeName: "cover" }) (or perhaps getCurrentTime({ range: "cover" })). This also allows .getCurrentTime() without arguments to continue being equivalent to .currentTime, as it should be.

[css-meeting-bot]

The CSS Working Group just discussed [scroll-animations-1] Animation.getCurrentTime is easily confused with Animation.currentTime, and agreed to the following:

• RESOLVED: Change getCurrentTime() parameter to a dict to satisfy Brian's/the TAG's concerns.

The full IRC log of that discussion

<TabAtkins> astearns: Previously decided on using getCurrentTime() with a position param
<TabAtkins> astearns: Comments since have noted that the tag prefers property bags
<TabAtkins> astearns: And the person who objects to the name would be okay with property bag instead of positional arg
<TabAtkins> astearns: So proposal is to change the previous resolution to use a property bag instead
<TabAtkins> astearns: Comments?
<TabAtkins> `getCurrentTime({ range: "cover" })` looks fine to me
<TabAtkins> astearns: Objections?
<TabAtkins> RESOLVED: Change getCurrentTime() parameter to a dict to satisfy Brian's/the TAG's concerns.

[yoavweiss]

Let me know if this requires a separate issue, but I was surprised to see that getCurrentTime returns a CSSNumericValue, rather than a DOMHighResTimestamp. I'm guessing that it'd also be worthwhile to hook up its processing/clamping to current high resolution time

[flackr]

Let me know if this requires a separate issue, but I was surprised to see that getCurrentTime returns a CSSNumericValue, rather than a DOMHighResTimestamp. I'm guessing that it'd also be worthwhile to hook up its processing/clamping to current high resolution time

Scroll timelines don't produce millisecond-based times, so we decided that we should future proof APIs with a CSSNumericValue which can represent the difference between milliseconds and percentages. We should still apply the same clamping to those millisecond values in the CSSNumericValue as we do for DOMHighResTimestamp.

[fantasai]

I believe this issue has been addressed, but there's some follow-up concerns in #8765