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

Call the view-transition page-visibility change steps #10137

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Feb 13, 2024

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543
and CSSWG resolution: w3c/csswg-drafts#9543 (comment)

(See WHATWG Working Mode: Changes for more details.)


/infrastructure.html ( diff )
/interaction.html ( diff )

This allows the CSS view-transitions spec to react to page visibility changes.
Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Spec text looks good with a nit. Since this is a tricky area, I have some test questions.

Can you confirm:

source Show resolved Hide resolved
@noamr
Copy link
Contributor Author

noamr commented Mar 26, 2024

Spec text looks good with a nit. Since this is a tricky area, I have some test questions.

Can you confirm:

  • It's tested that document's visibility state is updated before running the view transition page visibility change steps?

Yes! see https://github.com/web-platform-tests/wpt/blob/master/css/css-view-transitions/transition-in-hidden-page.html#L22

  • It's tested that view transition page visibility change steps come before firing visibilitychange?

Oops, testing the opposite :(
Corrected in web-platform-tests/wpt#45342

The order here doesn't matter and is not observable, as the screen orientation steps only apply when the document is visible and the view transition steps only apply when the document is hidden.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 26, 2024
- Change the order of operations when applying visibility change steps
- Use InvalidStateError when skipping transition due to visibility
- Use FromIfExist so that we don't create the VT supplementable
  unnecessarily

See whatwg/html#10137

Bug: 330533269
Bug: 331237574
Change-Id: I2f9e66aecf5da6db01bb4c65d3f0b4c39563c142
@noamr
Copy link
Contributor Author

noamr commented Mar 26, 2024

I've received feedback internally that perhaps we should move this to after firing visibilitychange, as the observable behavior is promise-based. That's what the current implementation+WPTs do. Any objection @domenic ?

@domenic
Copy link
Member

domenic commented Mar 27, 2024

Can you expand on the reasoning? It seems we have a medium-strong precedent that visibilitychange is last, so I'd like more explanation before we start breaking that, including reasoning on what types of features should go before vs. after so that we can know in the future where to put things. What kind of code are developers writing, and what is the impact on that code with before vs. after? Why is this case different than screen orientation, device posture, or Web NFC?

@noamr
Copy link
Contributor Author

noamr commented Mar 27, 2024

Can you expand on the reasoning? It seems we have a medium-strong precedent that visibilitychange is last, so I'd like more explanation before we start breaking that, including reasoning on what types of features should go before vs. after so that we can know in the future where to put things. What kind of code are developers writing, and what is the impact on that code with before vs. after? Why is this case different than screen orientation, device posture, or Web NFC?

For screen orientation and device posture, the visibility change steps queue a task to fire a different event. For WebNFC, some promises might be rejected which would be similar to this, but I'm not certain the order of operations is tested.

So in essence, the consistent way of going about it would be to skip the transition after firing the event, or running them before but internally queuing a task to resolve the promises. Perhaps the latter is cleaner, but in terms of order of operations it's the same.

@domenic
Copy link
Member

domenic commented Mar 27, 2024

Sorry ,that isn't enough detail for me. Can you help with

What kind of code are developers writing, and what is the impact on that code with before vs. after?

@noamr
Copy link
Contributor Author

noamr commented Mar 27, 2024

Sorry ,that isn't enough detail for me. Can you help with

What kind of code are developers writing, and what is the impact on that code with before vs. after?

Imagine this code:

const transition = document.startViewTransition();
onvisibilitychange = () => {
  if (document.hidden) { 
    cleanupVisualStuffWhenHidden();
  }
};

transition.finished.then(() => {
  cleanupTransitionSpecificStuff();
});

/* After these are called, the user switches tabs or minimizes the window */

With the current spec PR, cleanupTransitionSpecificStuff() will happen before cleanupVisualStuffWhenHidden().
If we swap the order, or queue a task to skip the transition, cleanupVisualStuffWhenHidden() will be called before cleanupTransitionSpecificStuff().

@domenic
Copy link
Member

domenic commented Apr 1, 2024

Thanks for explaining. Let's queue a task, then, as that gives implementations the most scheduling flexibility and is most consistent with what comes before.

noamr added a commit to noamr/csswg-drafts that referenced this pull request Apr 2, 2024
This doesn't change observable behavior, but makes the visibility reaction
consistent with other similar reactions.

See whatwg/html#10137
@noamr
Copy link
Contributor Author

noamr commented Apr 2, 2024

Thanks for explaining. Let's queue a task, then, as that gives implementations the most scheduling flexibility and is most consistent with what comes before.

Done. (w3c/csswg-drafts#10168)

@domenic domenic merged commit 9f50893 into whatwg:main Apr 2, 2024
2 checks passed
@noamr noamr deleted the vt-vis branch April 2, 2024 08:46
noamr added a commit to w3c/csswg-drafts that referenced this pull request Apr 8, 2024
…ask (#10168)

This doesn't change observable behavior, but makes the visibility reaction
consistent with other similar reactions.

See whatwg/html#10137
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants