From 6a9669b810ddfcae6c537165a438190ea1e7a4bc Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Mon, 4 Dec 2023 18:33:34 +0100 Subject: [PATCH] Increase consistency between navigation with and without ViewTransitions (#9279) * Greater consistency between navigations with and without ViewTransitions * hmpf * formatting and test fixing * An attempt to explain * fixed tests * explain the event listener * Update .changeset/nervous-beans-peel.md Co-authored-by: Nate Moore --------- Co-authored-by: Matthew Phillips Co-authored-by: Nate Moore --- .changeset/nervous-beans-peel.md | 6 + .../src/pages/long-page.astro | 6 + packages/astro/e2e/view-transitions.test.js | 3 + packages/astro/src/transitions/router.ts | 115 ++++++++---------- 4 files changed, 65 insertions(+), 65 deletions(-) create mode 100644 .changeset/nervous-beans-peel.md diff --git a/.changeset/nervous-beans-peel.md b/.changeset/nervous-beans-peel.md new file mode 100644 index 0000000000..aaf464db0b --- /dev/null +++ b/.changeset/nervous-beans-peel.md @@ -0,0 +1,6 @@ +--- +'astro': minor +--- + +Improves consistency between navigations with and without ``. See [#9279](https://github.com/withastro/astro/pull/9279) for more details. + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro index ebe918c680..aa7c8b5e59 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro @@ -51,3 +51,9 @@ import Layout from '../components/Layout.astro'; Libero id faucibus nisl tincidunt eget nullam non. Faucibus a pellentesque sit amet porttitor eget dolor. Posuere urna nec tincidunt praesent semper feugiat nibh sed. Suspendisse ultrices gravida dictum fusce. Porttitor eget dolor morbi non arcu. Neque egestas congue quisque egestas diam in. Suscipit tellus mauris a diam maecenas sed enim ut sem. Luctus accumsan tortor posuere ac. Tortor posuere ac ut consequat semper viverra. Egestas tellus rutrum tellus pellentesque eu tincidunt tortor aliquam nulla. Senectus et netus et malesuada fames ac turpis egestas. Sed libero enim sed faucibus turpis in eu mi bibendum. Sollicitudin tempor id eu nisl nunc mi. + diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index fafea3e697..bfc5d7d4a9 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -394,7 +394,10 @@ test.describe('View Transitions', () => { await expect(locator).toBeInViewport(); // Scroll back to top + // back returns immediately, but we need to wait for navigate() to complete + const waitForReady = page.waitForEvent('console'); await page.goBack(); + await waitForReady; locator = page.locator('#longpage'); await expect(locator).toBeInViewport(); diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index e5bf35b7fd..ce92fd6308 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -25,9 +25,7 @@ export const transitionEnabledOnThisPage = () => inBrowser && !!document.querySelector('[name="astro-view-transitions-enabled"]'); const samePage = (thisLocation: URL, otherLocation: URL) => - thisLocation.origin === otherLocation.origin && - thisLocation.pathname === otherLocation.pathname && - thisLocation.search === otherLocation.search; + thisLocation.pathname === otherLocation.pathname && thisLocation.search === otherLocation.search; // When we traverse the history, the window.location is already set to the new location. // This variable tells us where we came from @@ -418,16 +416,22 @@ async function transition( options: Options, historyState?: State ) { + // not ours + if (!transitionEnabledOnThisPage() || location.origin !== to.origin) { + location.href = to.href; + return; + } + const navigationType = historyState ? 'traverse' : options.history === 'replace' ? 'replace' : 'push'; - if (samePage(from, to) && !options.formData /* not yet: && to.hash*/) { - if (navigationType !== 'traverse') { - updateScrollPosition({ scrollX, scrollY }); - } + if (navigationType !== 'traverse') { + updateScrollPosition({ scrollX, scrollY }); + } + if (samePage(from, to) && !!to.hash) { moveToLocation(to, from, options, historyState); return; } @@ -447,61 +451,48 @@ async function transition( return; } - function pageMustReload(preparationEvent: TransitionBeforePreparationEvent) { - return ( - preparationEvent.to.hash === '' || - !samePage(preparationEvent.from, preparationEvent.to) || - preparationEvent.sourceElement instanceof HTMLFormElement - ); - } - async function defaultLoader(preparationEvent: TransitionBeforePreparationEvent) { - if (pageMustReload(preparationEvent)) { - const href = preparationEvent.to.href; - const init: RequestInit = {}; - if (preparationEvent.formData) { - init.method = 'POST'; - init.body = preparationEvent.formData; - } - const response = await fetchHTML(href, init); - // If there is a problem fetching the new page, just do an MPA navigation to it. - if (response === null) { - preparationEvent.preventDefault(); - return; - } - // if there was a redirection, show the final URL in the browser's address bar - if (response.redirected) { - preparationEvent.to = new URL(response.redirected); - } - - parser ??= new DOMParser(); - - preparationEvent.newDocument = parser.parseFromString(response.html, response.mediaType); - // The next line might look like a hack, - // but it is actually necessary as noscript elements - // and their contents are returned as markup by the parser, - // see https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString - preparationEvent.newDocument.querySelectorAll('noscript').forEach((el) => el.remove()); - - // If ViewTransitions is not enabled on the incoming page, do a full page load to it. - // Unless this was a form submission, in which case we do not want to trigger another mutation. - if ( - !preparationEvent.newDocument.querySelector('[name="astro-view-transitions-enabled"]') && - !preparationEvent.formData - ) { - preparationEvent.preventDefault(); - return; - } - - const links = preloadStyleLinks(preparationEvent.newDocument); - links.length && (await Promise.all(links)); - - if (import.meta.env.DEV) - await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to); - } else { - preparationEvent.newDocument = document; + const href = preparationEvent.to.href; + const init: RequestInit = {}; + if (preparationEvent.formData) { + init.method = 'POST'; + init.body = preparationEvent.formData; + } + const response = await fetchHTML(href, init); + // If there is a problem fetching the new page, just do an MPA navigation to it. + if (response === null) { + preparationEvent.preventDefault(); return; } + // if there was a redirection, show the final URL in the browser's address bar + if (response.redirected) { + preparationEvent.to = new URL(response.redirected); + } + + parser ??= new DOMParser(); + + preparationEvent.newDocument = parser.parseFromString(response.html, response.mediaType); + // The next line might look like a hack, + // but it is actually necessary as noscript elements + // and their contents are returned as markup by the parser, + // see https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString + preparationEvent.newDocument.querySelectorAll('noscript').forEach((el) => el.remove()); + + // If ViewTransitions is not enabled on the incoming page, do a full page load to it. + // Unless this was a form submission, in which case we do not want to trigger another mutation. + if ( + !preparationEvent.newDocument.querySelector('[name="astro-view-transitions-enabled"]') && + !preparationEvent.formData + ) { + preparationEvent.preventDefault(); + return; + } + + const links = preloadStyleLinks(preparationEvent.newDocument); + links.length && (await Promise.all(links)); + + if (import.meta.env.DEV) + await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to); } skipTransition = false; @@ -566,12 +557,6 @@ export async function navigate(href: string, options?: Options) { } return; } - - // not ours - if (!transitionEnabledOnThisPage()) { - location.href = href; - return; - } await transition('forward', originalLocation, new URL(href, location.href), options ?? {}); }