Skip to content
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

[css-view-transitions-1] Behaviour of the finished promise #7956

Closed
jakearchibald opened this issue Oct 26, 2022 · 7 comments
Closed

[css-view-transitions-1] Behaviour of the finished promise #7956

jakearchibald opened this issue Oct 26, 2022 · 7 comments
Labels
css-view-transitions-1 View Transitions; Bugs only

Comments

@jakearchibald
Copy link
Contributor

jakearchibald commented Oct 26, 2022

const transition = document.createViewTransition(updateDOMCallback);
await transition.finished;

There are two other promises:

  • domUpdated - resolves with the promise returned by updateDOMCallback.
  • ready - the transition has successfully prepared, and the pseudo-elements can now be targeted with JavaScript (eg web animation API). It will reject if the transition cannot be ready, either due to misconfiguration, or if updateDOMCallback returns a rejected promise.

I see two possible behaviours for finished:

Option 1: Fulfill on full uninterrupted completion of the transition

If the transition animation does not fully play through, finished rejects. This can happen if:

  • updateDOMCallback rejects - a failed DOM update is not transitioned
  • A misconfiguration means that, although the DOM successfully updated, a transition cannot be performed. For example, multiple elements have the same view-transition-name, when they're supposed to be unique.
  • The transition is aborted due to a newer transition.

Option 2: Fulfill on the 'end state' being visible to the user

The 'end state' being the state after a successful DOM change. This means finished will resolve with whatever happens first:

  • The transition plays through to completion
  • The transition is aborted, so it visually appears to skip to the end

finished will only reject if updateDOMCallback returns a rejected promise. In this case the DOM change failed, so it does not reach a valid end state.


I originally spec'd option 1, but I've been convinced on option 2, since this is slower in spirit to animation.finished, which will resolve even if the animation is skipped to the end (via animation.finish()).

@jakearchibald jakearchibald added the css-view-transitions-1 View Transitions; Bugs only label Oct 26, 2022
@ydaniv
Copy link
Contributor

ydaniv commented Oct 26, 2022

So this means that finished will be used only for transitions that played/seeked/skipped forward to completion, right?

What about animations that are reverse()'d during playing? Or seeked/skipped backwards to 0%? Will that count as rejection?
If the user is seeking the animation manually via script I guess they would need to either .reverse() the animation when it's back at 0% progress or .cancel() (the former seems more likely perhaps).
What should be the state of finished in such cases?

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Oct 26, 2022

There's no concept of 'reverse' for View Transitions.

@khushalsagar
Copy link
Member

There's no concept of 'reverse' for View Transitions.

Yeah, no such concept right now. There is some discussion on this at #7957 but for now ViewTransitions build on animation primitives which are either active or not.

With option 2, when does the finished promise resolve with respect to updateDOMCallback callback. Especially because we're leaning to dispatch that callback using a task in #7904:

  • Immediately when skip the page transition steps runs.
  • After the callback is invoked but doesn't wait for the domUpdated promise to settle.
  • After the domUpdated promise settles.

The WA-API doesn't have this problem since it synchronously reaches the end state if you call finish().

Maybe an example of how developers would rely on the finished promise always resolving will help with this. I figured if some code needs to be executed when we've reached the end state, i.e. DOM has been updated, it should wait on domUpdated promise.

@vmpstr vmpstr added the Agenda+ label Oct 27, 2022
@ydaniv
Copy link
Contributor

ydaniv commented Oct 28, 2022

finished will only reject if updateDOMCallback returns a rejected promise. In this case the DOM change failed, so it does not reach a valid end state.

Option 2 does sound better.

With option 2, when does the finished promise resolve with respect to updateDOMCallback callback.

That's a good point. I think it's important that finished is at least chained to promise returned from updateDOMCallback so that the author can predict the sequence.

There's no concept of 'reverse' for View Transitions.

Yeah, no such concept right now. There is some discussion on this at #7957 but for now ViewTransitions build on animation primitives which are either active or not.

Yes, question is how this API behaves when a user navigates back in mid transition? So that animations are reversed and 'end state' is actually previous view.

I figured if some code needs to be executed when we've reached the end state, i.e. DOM has been updated, it should wait on domUpdated promise.

It's possible that authors will await the finished promise so to not interfere with the animation, both waiting for idle visible state and possibly wishing not to degrade its perf.

@khushalsagar
Copy link
Member

khushalsagar commented Oct 28, 2022

I think it's important that finished is at least chained to promise returned from updateDOMCallback so that the author can predict the sequence.

That sounds fine. The author could technically wait on both promises in user code but if the common pattern will be to do something after the DOM has been updated and animation finished then resolving after domUpdatedPromise is settled (resolved or rejected) SGTM. @vmpstr @jakearchibald any concerns with this?

Yes, question is how this API behaves when a user navigates back in mid transition? So that animations are reversed and 'end state' is actually previous view.

For the same-documents script API this is completely in author's control, since the API itself doesn't have a concept of "back navigation":

  1. They could use existing APIs to reverse all the UA generated animations and use the finished API to update the DOM back to the old state.
  2. Or call skipTransition() which will draw at least one frame with the final state and then initiate a new transition.

(1) seems like the nicer default behaviour so maybe it makes sense to add an API to do this automatically. We'd likely also need to do it automatically for MPA. @ydaniv this seems unrelated to this issue. Would it be ok to discuss this on #7957?

@khushalsagar
Copy link
Member

Proposed Resolution: In skip the page transition, the finished promise resolves after the domUpdated resolves, or rejects after the domUpdated rejects (with the same error).

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-view-transitions-1] Behaviour of the finished promise, and agreed to the following:

  • RESOLVED: Accept proposal
The full IRC log of that discussion <fantasai> Topic: [css-view-transitions-1] Behaviour of the finished promise
<fantasai> github: https://github.com//issues/7956
<fantasai> JakeA: When you start a view transition, you get an object back that has some promises that you can use to track various things
<fantasai> JakeA: one of these is the "finished" promise
<fantasai> JakeA: question is, what should happen if the DOM change is successful, but the transition is not?
<fantasai> JakeA: transition could not be successful in the case of misconfiguration, e.g. duplicate IDs
<fantasai> JakeA: could also mean that transition is abandoned halfway through, because more important transition coming in
<fantasai> JakeA: Originally spec would reject unless the transition gets to final frame
<fantasai> JakeA: goes through the whole duration of the transition
<fantasai> JakeA: Proposing that it finishes whenever it gets to the end state
<fantasai> JakeA: we were influenced by animation.finish()
<fantasai> JakeA: and animation.finished which will resolve
<flackr> +1
<fantasai> JakeA: Desire to change this came after showing them the APIs, and asking them to guess what it would do
<fantasai> JakeA: that seemed to be where people were leaning :)
<fantasai> Rossen_: +1 for taking the time
<fantasai> Rossen_: and alignment with animations is awesome
<fantasai> Rossen_: any objections?
<bramus> +1
<fantasai> RESOLVED: Accept proposal
<fantasai> JakeA: Yes, animation needs to have stopped (even if not completed), and user is looking at new state

khushalsagar added a commit to khushalsagar/csswg-drafts that referenced this issue Nov 10, 2022
Applies resolutions from following issues:

- w3c#7956
- w3c#7874
- w3c#7812
vmpstr added a commit that referenced this issue Nov 10, 2022
[css-view-transitions-1] Update spec for resolved issues (#7956, #7874, #7812)
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 11, 2022
The finished promise should reflect the status of domUpdated if the
transition is skipped before animations could finish.

See resolution at w3c/csswg-drafts#7956.

Change-Id: If340d63587d87237587fc3aa3de98f742d46218e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 14, 2022
The finished promise should reflect the status of domUpdated if the
transition is skipped before animations could finish.

See resolution at w3c/csswg-drafts#7956.

Change-Id: If340d63587d87237587fc3aa3de98f742d46218e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
The finished promise should reflect the status of domUpdated if the
transition is skipped before animations could finish.

See resolution at w3c/csswg-drafts#7956.

Change-Id: If340d63587d87237587fc3aa3de98f742d46218e
jakearchibald pushed a commit to jakearchibald/csswg-drafts that referenced this issue Jan 16, 2023
Applies resolutions from following issues:

- w3c#7956
- w3c#7874
- w3c#7812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-view-transitions-1 View Transitions; Bugs only
Projects
None yet
Development

No branches or pull requests

6 participants