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

Technical review: Document cross-document view transitions #32723

Closed

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Mar 18, 2024

Note: This technical review is now completed and approved. For the follow-on editorial review, see #34118


Description

Chrome 123/124 includes features required for developers to create cross-document View Transitions. This includes:

This PR adds content for all of these. In addition, it:

  • Updates the View Transitions API landing page to cover these new features.
  • Moves the guide content to a separate "Using the View Transitions API" article.
  • Makes some updates to existing view transitions pages, so that they work for SPA and MPA use cases.

Related ChromeStatus entries:

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners March 18, 2024 13:41
@chrisdavidmills chrisdavidmills requested review from Elchi3 and bsmth and removed request for a team March 18, 2024 13:41
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs labels Mar 18, 2024
@chrisdavidmills chrisdavidmills marked this pull request as draft March 18, 2024 13:42
@github-actions github-actions bot added the size/l [PR only] 501-1000 LoC changed label Mar 18, 2024
Copy link
Contributor

github-actions bot commented Mar 18, 2024

Preview URLs (32 pages)
Flaws (7)

Note! 29 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/HTML_DOM_API
Title: The HTML DOM API
Flaw count: 5

  • macros:
    • /en-US/docs/Web/API/HTMLDirectoryElement does not exist
    • /en-US/docs/Web/API/HTMLFrameElement does not exist
    • /en-US/docs/Web/API/HTMLIsIndexElement does not exist
    • /en-US/docs/Web/API/External does not exist
    • /en-US/docs/Web/API/ApplicationCache does not exist

URL: /en-US/docs/Web/API/NavigationActivation
Title: NavigationActivation
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/Navigation/activation does not exist

URL: /en-US/docs/Web/HTML/Attributes/rel
Title: HTML attribute: rel
Flaw count: 1

  • broken_links:
    • No need for the pathname in anchor links if it's the same page
External URLs (22)

URL: /en-US/docs/Web/API/View_Transitions_API
Title: View Transitions API


URL: /en-US/docs/Web/API/View_Transitions_API/Using
Title: Using the View Transitions API


URL: /en-US/docs/Web/API/PageSwapEvent
Title: PageSwapEvent


URL: /en-US/docs/Web/API/Window
Title: Window


URL: /en-US/docs/Web/API/Window/pageswap_event
Title: Window: pageswap event


URL: /en-US/docs/Web/API/Window/pagereveal_event
Title: Window: pagereveal event


URL: /en-US/docs/Web/API/NavigationActivation
Title: NavigationActivation


URL: /en-US/docs/Web/API/PageRevealEvent
Title: PageRevealEvent


URL: /en-US/docs/Web/API/Document/startViewTransition
Title: Document: startViewTransition() method


URL: /en-US/docs/Web/CSS/@view-transition
Title: @view-transition


URL: /en-US/docs/Web/CSS/::view-transition-group
Title: ::view-transition-group


URL: /en-US/docs/Web/CSS/::view-transition-old
Title: ::view-transition-old


URL: /en-US/docs/Web/CSS/::view-transition-new
Title: ::view-transition-new

(comment last updated: 2024-06-13 14:41:53)

@github-actions github-actions bot added the Content:HTML Hypertext Markup Language docs label Mar 20, 2024
@bsmth bsmth removed their request for review March 20, 2024 12:57
@github-actions github-actions bot added size/xl [PR only] >1000 LoC changed and removed size/l [PR only] 501-1000 LoC changed labels Mar 21, 2024
@chrisdavidmills chrisdavidmills marked this pull request as ready for review March 22, 2024 15:57
@chrisdavidmills chrisdavidmills requested a review from a team as a code owner March 22, 2024 15:57
@chrisdavidmills chrisdavidmills removed the request for review from a team March 22, 2024 15:57
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
chrisdavidmills and others added 8 commits May 24, 2024 16:11
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@bramus bramus left a comment

Choose a reason for hiding this comment

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

Apologies for the delay, but here’s a review :)

files/en-us/web/api/navigationactivation/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/navigationactivation/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/navigationactivation/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/navigationactivation/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/pagerevealevent/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pagereveal_event/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pagereveal_event/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pageswap_event/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pageswap_event/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pageswap_event/index.md Outdated Show resolved Hide resolved
@chrisdavidmills chrisdavidmills requested a review from bramus June 12, 2024 18:59
Copy link
Contributor

@bramus bramus left a comment

Choose a reason for hiding this comment

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

More code comments. For the one in pageswap I await e.viewTransition.finished, for the one in pagereveal it’s e.viewTransition.ready.

files/en-us/web/api/pageswapevent/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/pageswapevent/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/view_transitions_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/view_transitions_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pageswap_event/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/pageswap_event/index.md Outdated Show resolved Hide resolved
@chrisdavidmills
Copy link
Contributor Author

Thanks again @bramus. I made a couple more updates in light of your further comments. Let me know if you spot anything else. If not, then we can probably move this forward to editorial review stage.

One thing (which is not a blocker) — I updated the comment about BFCache in the code to:

(this is to deal with BFCache)

Can you explain what this comment means? Because readers will wonder. I'd like to add an explanation. I think I know what this is getting at, but I want to make sure.

@chrisdavidmills chrisdavidmills requested a review from bramus June 13, 2024 11:24
Copy link
Contributor

@bramus bramus left a comment

Choose a reason for hiding this comment

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

Approved.

@bramus
Copy link
Contributor

bramus commented Jun 13, 2024

One thing (which is not a blocker) — I updated the comment about BFCache in the code to:

(this is to deal with BFCache)

Can you explain what this comment means? Because readers will wonder. I'd like to add an explanation. I think I know what this is getting at, but I want to make sure.

On page1, in pageswap I am setting the v-t-name values on the fly on the page. This changed state is what gets persisted in BFCache. If I were to hit the back button from page2 back to page1, the pagereveal from page1 would try and set the same v-t-name values, resulting in a conflict because now to different elements would have the same v-t-name. Therefore I have to make sure the names are unset before the page goes into BFCache.

@chrisdavidmills
Copy link
Contributor Author

OK, closing this PR. Next stage is to create a new PR based on the same branch to contain the editorial review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs Content:HTML Hypertext Markup Language docs Content:WebAPI Web API docs size/xl [PR only] >1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants