w3c / csswg-drafts

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

[scroll-animations] Should animation events fire every time active range is left / reentered? #4324

Closed flackr closed 1 year ago

flackr commented 4 years ago

web-animations 4.4.19.2 suggests that a finish event (i.e. animationend) is queued whenever the animation enters the finished play state. Similarly a cancel event (i.e. animationcancel) is queued whenever the animation enters the idle play state.

ScrollTimeline based animations would technically enter one of these states any time the user scrolls past the active range. Does this mean we should be emitting animationend / animationcancel every time you leave the active range and animationstart everytime it is reentered? It's possible developers could use this to trigger post animation range updates.

If so, what about scrolling backwards when current time <= 0 but the animation's effective playback rate is still > 0? Should this generate a finish event since effectively we were playing the animation in reverse?

birtles commented 4 years ago

Good questions. Regarding the finish events, I think the way the CSS Animations Level 2 event dispatch is defined would work pretty well in these cases. Since Web Animations doesn't have a start event, however, it's tricky to know what to do in the scrolling backwards case. Maybe we need to add that event.

Regarding cancel events, I also agree that we probably shouldn't be dispatching those. We probably need to tweak either the definition of the idle state or cancel event dispatch so that we get finish events instead of cancel events, probably the former. I suspect this overlaps with https://github.com/w3c/csswg-drafts/issues/2066 somewhat.

flackr commented 1 year ago

Since we have updated the spec such that when you play scroll linked animations they get a start time of 0 at step 7 of web-animations-1 4.4.8 Playing an animation, we shouldn't have to worry about cancel events being dispatched anymore since the animation would not be considered idle due to its defined start time.

Proposed resolutions:

  1. Animation events fire for scroll linked animations as defined by [web-animations-1 4.4.18.3],(https://www.w3.org/TR/web-animations-1/#animation-playback-event-types) and css-animations-2 4.1.
  2. (Optional) Introduce a new event for web-animations which is dispatched when an animation goes from the idle or before phase to active phase equivalent to the CSS animationstart event.
css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [scroll-animations] Should animation events fire every time active range is left / reentered?, and agreed to the following:

The full IRC log of that discussion <Rossen_> https://github.com/w3c/csswg-drafts/issues/4324#issuecomment-1325707643
<fantasai> Proposal: https://github.com/w3c/csswg-drafts/issues/4324
<fantasai> Proposal: https://github.com/w3c/csswg-drafts/issues/4324#issuecomment-1325707643
<dael> flackr: An open question we had lingering about with a scroll linked animation you can repeatedly enter before or after phase by scrolling. As defined this fires animation events. Should it?
<dael> flackr: birtles and I chatted and we think it seems reasonable to fire events as define. Prop is events fire for scroll linked animations as specified
<dael> flackr: Second part we can take up separately
<dael> Rossen_: I see some back and forth on this. Anyone want to express other opinions?
<dael> Rossen_: Taking the silence as no additional opinions
<dael> flackr: Animation events fire for scroll linked animations as defined by web animations spec
<dael> Rossen_: Objections?
<dael> RESOLVED: Animation events fire for scroll linked animations as defined by web animations spec
<dael> flackr: Second part, CSS animations have an animation start event. Web animations have a ready promise which is not quite the same. Ready would resolve once.
<dael> flackr: Would it be useful to add a start event that developers could use to trigger on same condition. Every time you go from before animation is active to when it's active
<Rossen_> q?
<dael> birtles: I think you need something if it's possible to finish and then go unfinished good for authors to detect. Don't know it's a start event b/c that would relate to the delay. Maybe just an active event. Need to define some way for authors to know it goes from finished to not finished
<bramus> q+
<dael> flackr: Sounds like something we should come up with ideas and bring back.
<dael> flackr: Let's stick with first resolution and move on
<Rossen_> ack bramus
<dael> bramus: Wondering how that would work with various phases. I'm okay if we come up with some ideas to discuss
fantasai commented 1 year ago

@flackr Reviewing the logs, I'm not clear on what edits are required here. It seems like there should be some cross-references to https://drafts.csswg.org/css-animations/#events but it seems unclear to me what happens when you scroll backwards (or use the Web Animations API or a UA-provided control to reverse animations): do you fire animationstart before animationend, or do you fire animationend before animationstart?

Also it seems there was a suggestion to propose a new event, but that needs follow-up.

flackr commented 1 year ago

@fantasai You're right that the backwards direction is awkward. I put together a little demo (view in chrome canary with experimental web platform features enabled) to test what events are fired for JS and CSS animations. As discussed in this issue JS animations only currently have a finish event so the start is not observable:

https://jsbin.com/xuwebet/edit?css,js,output

The CSS events fire based on phase changes outlined in css-animations-2 4.1 Event dispatch. This seems nice from a developer standpoint where it fires animationstart every time you go from outside the active interval to inside of it, and animationend everytime you leave the active interval.

Without spec changes, the behavior for JS animations is that finish fires only on moving forwards past the end scroll offset. This is because the finish state is explicitly the time after the animation end which doesn't change when you change scrolling directions (unless we also considered a change the scroll direction a modification of the sign of the playbackRate). I would prefer we either change the event dispatch to align with the way CSS animation events fire or introduce new JS events that align with the CSS event model. @birtles WDYT? I suspect adding new events aligned with the dispatch of the CSS animation events is less risky and wouldn't be confusing.

Assuming we do fire events bidirectionally (as the CSS events currently do), the developer can use the currentTime / elapsedTime attribute of the event to determine whether it was for the forwards or backwards direction. Another thing I noticed is the elapsed time on the css-animation events is currently exposing our internal timestampas which should be CSSNumericValue instead.

flackr commented 1 year ago

Sorry for the long blurb, the TLDR is this issue was about whether we need to change event dispatch for scroll animations. The answer is that I don't think we need to - unless you feel strongly that finish should work bidirectionally. I think that there is an implication the finish event is equivalent to what you get when you call finish() suggesting that the way it works now is expected.

I'll open a separate issue for dispatching web-animations events which are aligned with the CSS ones.

flackr commented 1 year ago

Opened #8544 to discuss adding web-animations events corresponding to animationstart/animationend.

birtles commented 1 year ago

Followed up over in #8544.

flackr commented 1 year ago

@fantasai Do you agree we can close this as is? No spec edits should be needed and we can discuss start/end events in #8544.

fantasai commented 1 year ago

@flackr I think it needs some clarifying cross-references, which I added in https://github.com/w3c/csswg-drafts/commit/26a6269f0012779fa3364c17fc2b9f8cb21a479c

I also added the following note:

When scrolling backwards, the animationstart event will fire at the end of the active interval, and the animationend event will fire at the start of the active interval. However, since the finish event is about entering the finished play state, it only fires when scrolling forwards.

So the two questions are: does this correctly describe what's going to happen? :) And is this what we want?

flackr commented 1 year ago

The added clarifying text looks good and describes the common case. Note that it's slightly more nuanced since we won't fire if you scroll to exactly the start / end. It requires that you have scrolled before / after the active interval.

I don't think there's any reason to change the behavior - I think the way animationstart/animationend work makes sense and could reasonably be used by developers to add / remove additional effects when the active scroll range is entered / left.