Skip to content

[css-view-transitions-1] Swap the order between setting the phase and calling the update callback #10826

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

Merged
merged 3 commits into from
Sep 5, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions css-view-transitions-1/Overview.bs
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,8 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;

1. [=Assert=]: |transition|'s [=ViewTransition/phase=] is "`done`", or before "`update-callback-called`".

1. If |transition|'s [=ViewTransition/phase=] is not "`done`", then set |transition|'s [=ViewTransition/phase=] to "`update-callback-called`".
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May as well move it further up to keep the code using callbackPromise together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done


1. Let |callbackPromise| be null.

1. If |transition|'s [=ViewTransition/update callback=] is null,
Expand All @@ -1606,8 +1608,6 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;

1. Otherwise, set |callbackPromise| to the result of [=/invoking=] |transition|'s [=ViewTransition/update callback=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "otherwise" here correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, fixed


1. If |transition|'s [=ViewTransition/phase=] is not "`done`", then set |transition|'s [=ViewTransition/phase=] to "`update-callback-called`".

1. Let |fulfillSteps| be to following steps:
1. [=Resolve=] |transition|'s [=ViewTransition/update callback done promise=] with undefined.

Expand Down Expand Up @@ -1986,6 +1986,7 @@ Changes from <a href="https://www.w3.org/TR/2023/WD-css-view-transitions-1-20230
* Scope view transition names to matching tree context. See <a href="https://github.com/w3c/csswg-drafts/issues/10145">issue 10145</a>.
* Fix scoping to match name instead of element. See <a href="https://github.com/w3c/csswg-drafts/issues/10145">issue 10145</a>.
* Add a rendering characteristics note about out-of-viewport elements. See <a href="https://github.com/w3c/csswg-drafts/issues/8282">issue 8282</a>.
* Swap the order of invoking the update callback and setting the phase. See <a href="https://github.com/w3c/csswg-drafts/issues/10822">issue 10822</a>.

<h3 id="changes-since-2022-05-25">
Changes from <a href="https://www.w3.org/TR/2023/WD-css-view-transitions-1-20230525/">2022-05-25 Working Draft</a>
Expand Down