-
Notifications
You must be signed in to change notification settings - Fork 714
[css-view-transitions-1] Refactor timings of updateCallbackDone
and related
#9774
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
Conversation
css-view-transitions-1/Overview.bs
Outdated
|
||
- If the promise was fulfilled, then return undefined. | ||
1. If |transition|'s [=ViewTransition/phase=] is "`done`", then [=resolve=] |transition|'s [=ViewTransition/update callback done promise=] with undefined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolving multiple times is a no-op right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When would we resolve multiple times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I guess if setting up the pseudo-elements fail... yea, https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promise-resolve-functions step 5
We might not even need the if
, we can simply resolve and rely on that behavior. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the spec here!
css-view-transitions-1/Overview.bs
Outdated
1. [=Skip the view transition=] |transition| with |reason|. | ||
|
||
* If the promise was fulfilled, then [=activate view transition|activate=] |transition|. | ||
Note: A task is queued here because the texture read back in [=capturing the image=] may be async, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind restoring the position of this note? Nicer to have this next to where we post the task. Otherwise its part of this sub-point which is less clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Note: this change is not a behavior change, but rather a clarification of existing expected behavior.
updateCallbackDone
is always resolved after the new state capture but before theready
promise. (This is current behavior, but not explicit in the current spec).done
phase.Closes #9762