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

Transition only between pages where both have ViewTransitions enabled #8441

Merged
merged 12 commits into from
Sep 7, 2023
Merged

Transition only between pages where both have ViewTransitions enabled #8441

merged 12 commits into from
Sep 7, 2023

Conversation

martrapp
Copy link
Member

@martrapp martrapp commented Sep 6, 2023

Changes

Before:
The history could contain states which are not handled by the client-side router when it dies during a visit to pages without <ViewTransitions>. The client-side router dies when a page w/o <ViewTransitions> is (re)loaded (no swap). The load deletes the router's modules. This is not a problem if the user navigates back to a page with <ViewTransitions> and this is reloaded. But if the browser history contains an entry for that page that was originally created by the router, the browser expects the now-deceased router to force the reload for that history entry. This results in a broken backward navigation.

Change:
With this change, new history entries and (view transitions) will only be created if the target page also defines <ViewTransitions>. This way the browser's backward navigation will always restore the router in time, even if lost on pages without <ViewTransitions>.

Testing

Added e2e test to prove that a killed router is revived correctly.
Adjusted existing tests because navigation to pages without <ViewTransition> leads to reloads.

Docs

n/a. bug fix

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Sep 6, 2023
@changeset-bot
Copy link

changeset-bot bot commented Sep 6, 2023

🦋 Changeset detected

Latest commit: 0fedb9a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@martrapp martrapp changed the title Only transition between pages where both have ViewTransitions enabled Transition only between pages where both have ViewTransitions enabled Sep 7, 2023
@matthewp
Copy link
Contributor

matthewp commented Sep 7, 2023

Looks great! Should note that we checked <meta name="view-transition" content="same-origin" /> and it behaves the same way as this PR. This prevents bugs were you can get "stuck" going forward to pages without VT and then trying to come back.

@matthewp matthewp merged commit f66053a into withastro:main Sep 7, 2023
@astrobot-houston astrobot-houston mentioned this pull request Sep 7, 2023
@martrapp martrapp deleted the rescue-router branch September 7, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants