diff options
3 files changed, 86 insertions, 79 deletions
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/listener-layout.astro b/packages/astro/e2e/fixtures/view-transitions/src/components/listener-layout.astro index 9f6265335..12c8d4469 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/components/listener-layout.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/components/listener-layout.astro @@ -10,25 +10,20 @@ import { ViewTransitions } from 'astro:transitions'; <slot/> <script> document.addEventListener("astro:after-swap", () => { - document.querySelector("p").addEventListener("transitionstart", () => { - console.info("transitionstart"); - }); document.documentElement.setAttribute("class", "blue"); + document.documentElement.setAttribute("animations", "" + document.getAnimations().length); }); - document.dispatchEvent(new Event("astro:after-swap")); </script> </body> <style> p { - transition: background-color 1s; + transition: color 1s; } p { - background-color: #0ee; color: red; } .blue p { - background-color: #ee0; - color: blue; + color: #0000ff; } </style> </html> diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index a378df7c5..37929073a 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import { testFactory } from './test-utils.js'; +import { testFactory, waitForHydrate } from './test-utils.js'; const test = testFactory({ root: './fixtures/view-transitions/' }); @@ -231,25 +231,37 @@ test.describe('View Transitions', () => { }); test('No page rendering during swap()', async ({ page, astro }) => { - let transitions = 0; - page.on('console', (msg) => { - if (msg.type() === 'info' && msg.text() === 'transitionstart') ++transitions; - }); + // This has been a problem with theme switchers (e.g. for drakmode) + // Swap() should not trigger any page renders and give users the chance to + // correct attributes in the astro:after-swap handler before they become visible + + // This test uses a CSS animation to detect page rendering + // The test succeeds if no additional animation beside those of the + // view transition is triggered during swap() - // Go to page 1 await page.goto(astro.resolveUrl('/listener-one')); let p = page.locator('#totwo'); await expect(p, 'should have content').toHaveText('Go to listener two'); - // on load a CSS transition is started triggered by a class on the html element - expect(transitions).toBeLessThanOrEqual(1); - const transitionsBefore = transitions; + + // setting the blue class on the html element triggers a CSS animation + let animations = await page.evaluate(async () => { + document.documentElement.classList.add('blue'); + return document.getAnimations(); + }); + expect(animations.length).toEqual(1); + // go to page 2 await page.click('#totwo'); p = page.locator('#toone'); await expect(p, 'should have content').toHaveText('Go to listener one'); - // swap() resets that class, the after-swap listener sets it again. - // the temporarily missing class must not trigger page rendering - expect(transitions).toEqual(transitionsBefore); + // swap() resets the "blue" class, as it is not set in the static html of page 2 + // The astro:after-swap listener (defined in the layout) sets it to "blue" again. + // The temporarily missing class must not trigger page rendering. + + // When the after-swap listener starts, no animations should be running + // after-swap listener sets animations to document.getAnimations().length + // and we expect this to be zero + await expect(page.locator('html')).toHaveAttribute('animations', '0'); }); test('click hash links does not do navigation', async ({ page, astro }) => { @@ -663,10 +675,11 @@ test.describe('View Transitions', () => { // go to external page await page.click('#click-redirect-external'); // doesn't work for playwright when we are too fast - await page.waitForTimeout(1000); p = page.locator('h1'); - await expect(p, 'should have content').toBeVisible(); + await expect(p, 'should have content').toBeVisible(); + await page.waitForURL('http://example.com'); + await page.waitForFunction((arr) => arr.length === 2, loads); expect(loads.length, 'There should be 2 page loads').toEqual(2); }); @@ -696,7 +709,8 @@ test.describe('View Transitions', () => { await page.goto(astro.resolveUrl('/client-only-three')); let msg = page.locator('#name'); await expect(msg).toHaveText('client-only-three'); - await page.waitForTimeout(400); // await hydration + await waitForHydrate(page, page.getByText('Vue')); + await waitForHydrate(page, page.getByText('Svelte')); let styles = await page.locator('style').all(); expect(styles.length).toEqual(totalExpectedStyles_page_three); diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 1bbbc85a1..583bf7dfa 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -12,7 +12,8 @@ type Events = 'astro:page-load' | 'astro:after-swap'; // only update history entries that are managed by us // leave other entries alone and do not accidently add state. -const persistState = (state: State) => history.state && history.replaceState(state, ''); +const updateScrollPosition = (positions: { scrollX: number; scrollY: number }) => + history.state && history.replaceState({ ...history.state, ...positions }, ''); const inBrowser = import.meta.env.SSR === false; @@ -58,11 +59,13 @@ let currentHistoryIndex = 0; if (inBrowser) { if (history.state) { - // we reloaded a page with history state + // Here we reloaded a page with history state // (e.g. history navigation from non-transition page or browser reload) currentHistoryIndex = history.state.index; scrollTo({ left: history.state.scrollX, top: history.state.scrollY }); } else if (transitionEnabledOnThisPage()) { + // This page is loaded from the browser addressbar or via a link from extern, + // it needs a state in the history history.replaceState({ index: currentHistoryIndex, scrollX, scrollY, intraPage: false }, ''); } } @@ -122,12 +125,6 @@ function getFallback(): Fallback { return 'animate'; } -function markScriptsExec() { - for (const script of document.scripts) { - script.dataset.astroExec = ''; - } -} - function runScripts() { let wait = Promise.resolve(); for (const script of Array.from(document.scripts)) { @@ -156,7 +153,9 @@ function isInfinite(animation: Animation) { return style.animationIterationCount === 'infinite'; } -const updateHistoryAndScrollPosition = (toLocation: URL, replace: boolean, intraPage: boolean) => { +// Add a new entry to the browser history. This also sets the new page in the browser addressbar. +// Sets the scroll position according to the hash fragment of the new location. +const moveToLocation = (toLocation: URL, replace: boolean, intraPage: boolean) => { const fresh = !samePage(toLocation); let scrolledToTop = false; if (toLocation.href !== location.href) { @@ -190,6 +189,32 @@ const updateHistoryAndScrollPosition = (toLocation: URL, replace: boolean, intra } }; +function stylePreloadLinks(newDocument: Document) { + const links: Promise<any>[] = []; + for (const el of newDocument.querySelectorAll('head link[rel=stylesheet]')) { + // Do not preload links that are already on the page. + if ( + !document.querySelector( + `[${PERSIST_ATTR}="${el.getAttribute( + PERSIST_ATTR + )}"], link[rel=stylesheet][href="${el.getAttribute('href')}"]` + ) + ) { + const c = document.createElement('link'); + c.setAttribute('rel', 'preload'); + c.setAttribute('as', 'style'); + c.setAttribute('href', el.getAttribute('href')!); + links.push( + new Promise<any>((resolve) => { + ['load', 'error'].forEach((evName) => c.addEventListener(evName, resolve)); + document.head.append(c); + }) + ); + } + } + return links; +} + // replace head and body of the windows document with contents from newDocument // if !popstate, update the history entry and scroll position according to toLocation // if popState is given, this holds the scroll position for history navigation @@ -204,9 +229,8 @@ async function updateDOM( // Check for a head element that should persist and returns it, // either because it has the data attribute or is a link el. // Returns null if the element is not part of the new head, undefined if it should be left alone. - const persistedHeadElement = (el: HTMLElement): Element | null | undefined => { + const persistedHeadElement = (el: HTMLElement): Element | null => { const id = el.getAttribute(PERSIST_ATTR); - if (id === '') return undefined; const newEl = id && newDocument.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); if (newEl) { return newEl; @@ -293,7 +317,7 @@ async function updateDOM( // from the new document and leave the current node alone if (newEl) { newEl.remove(); - } else if (newEl === null) { + } else { // Otherwise remove the element in the head. It doesn't exist in the new page. el.remove(); } @@ -324,35 +348,13 @@ async function updateDOM( if (popState) { scrollTo(popState.scrollX, popState.scrollY); // usings 'auto' scrollBehavior } else { - updateHistoryAndScrollPosition(toLocation, options.history === 'replace', false); + moveToLocation(toLocation, options.history === 'replace', false); } triggerEvent('astro:after-swap'); }; - // Wait on links to finish, to prevent FOUC - const links: Promise<any>[] = []; - for (const el of newDocument.querySelectorAll('head link[rel=stylesheet]')) { - // Do not preload links that are already on the page. - if ( - !document.querySelector( - `[${PERSIST_ATTR}="${el.getAttribute( - PERSIST_ATTR - )}"], link[rel=stylesheet][href="${el.getAttribute('href')}"]` - ) - ) { - const c = document.createElement('link'); - c.setAttribute('rel', 'preload'); - c.setAttribute('as', 'style'); - c.setAttribute('href', el.getAttribute('href')!); - links.push( - new Promise<any>((resolve) => { - ['load', 'error'].forEach((evName) => c.addEventListener(evName, resolve)); - document.head.append(c); - }) - ); - } - } + const links = stylePreloadLinks(newDocument); links.length && (await Promise.all(links)); if (fallback === 'animate') { @@ -363,12 +365,9 @@ async function updateDOM( .getAnimations() .filter((a) => !currentAnimations.includes(a) && !isInfinite(a)); const finished = Promise.all(newAnimations.map((a) => a.finished)); - const fallbackSwap = () => { - swap(); - document.documentElement.dataset.astroTransitionFallback = 'new'; - }; await finished; - fallbackSwap(); + swap(); + document.documentElement.dataset.astroTransitionFallback = 'new'; } else { swap(); } @@ -427,7 +426,6 @@ async function transition( // skip this for the moment as it tends to stop fallback animations // document.documentElement.removeAttribute('data-astro-transition'); await runScripts(); - markScriptsExec(); onPageLoad(); announce(); } @@ -460,7 +458,7 @@ export function navigate(href: string, options?: Options) { // but we want to handle prevent reload on navigation to the same page // Same page means same origin, path and query params (but maybe different hash) if (location.origin === toLocation.origin && samePage(toLocation)) { - updateHistoryAndScrollPosition(toLocation, options?.history === 'replace', true); + moveToLocation(toLocation, options?.history === 'replace', true); } else { // different origin will be detected by fetch transition('forward', toLocation, options ?? {}); @@ -509,20 +507,21 @@ function onPopState(ev: PopStateEvent) { } } +// There's not a good way to record scroll position before a back button. +// So the way we do it is by listening to scrollend if supported, and if not continuously record the scroll position. +const onScroll = () => { + updateScrollPosition({ scrollX, scrollY }); +}; + if (inBrowser) { if (supportsViewTransitions || getFallback() !== 'none') { addEventListener('popstate', onPopState); addEventListener('load', onPageLoad); - // There's not a good way to record scroll position before a back button. - // So the way we do it is by listening to scrollend if supported, and if not continuously record the scroll position. - const updateState = () => { - persistState({ ...history.state, scrollX, scrollY }); - }; - - if ('onscrollend' in window) addEventListener('scrollend', updateState); - else addEventListener('scroll', throttle(updateState, 300)); - - markScriptsExec(); + if ('onscrollend' in window) addEventListener('scrollend', onScroll); + else addEventListener('scroll', throttle(onScroll, 300)); + } + for (const script of document.scripts) { + script.dataset.astroExec = ''; } } @@ -549,12 +548,11 @@ async function prepareForClientOnlyComponents(newDocument: Document, toLocation: const viteIds = [...nextHead.querySelectorAll(`style[${VITE_ID}]`)].map((style) => style.getAttribute(VITE_ID) ); - // Mark styles of the current head as persistent - // if they come from hydration and not from the newDocument + // Copy required styles to the new document if they are from hydration. viteIds.forEach((id) => { const style = document.head.querySelector(`style[${VITE_ID}="${id}"]`); if (style && !newDocument.head.querySelector(`style[${VITE_ID}="${id}"]`)) { - style.setAttribute(PERSIST_ATTR, ''); + newDocument.head.appendChild(style); } }); } |