diff --git a/.changeset/chilled-shoes-fail.md b/.changeset/chilled-shoes-fail.md new file mode 100644 index 000000000..1567ecca3 --- /dev/null +++ b/.changeset/chilled-shoes-fail.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +ViewTransitions: Fixes in the client-side router diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index be312b1bf..12dfe0f4f 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -20,15 +20,6 @@ const { fallback = 'animate' } = Astro.props as Props; type Events = 'astro:load' | 'astro:beforeload'; const persistState = (state: State) => history.replaceState(state, ''); - - // The History API does not tell you if navigation is forward or back, so - // you can figure it using an index. On pushState the index is incremented so you - // can use that to determine popstate if going forward or back. - let currentHistoryIndex = history.state?.index || 0; - if (!history.state) { - persistState({ index: currentHistoryIndex, scrollY: 0 }); - } - const supportsViewTransitions = !!document.startViewTransition; const transitionEnabledOnThisPage = () => !!document.querySelector('[name="astro-view-transitions-enabled"]'); @@ -36,6 +27,14 @@ const { fallback = 'animate' } = Astro.props as Props; const onload = () => triggerEvent('astro:load'); const PERSIST_ATTR = 'data-astro-transition-persist'; + // The History API does not tell you if navigation is forward or back, so + // you can figure it using an index. On pushState the index is incremented so you + // can use that to determine popstate if going forward or back. + let currentHistoryIndex = history.state?.index || 0; + if (!history.state && transitionEnabledOnThisPage()) { + persistState({ index: currentHistoryIndex, scrollY: 0 }); + } + const throttle = (cb: (...args: any[]) => any, delay: number) => { let wait = false; // During the waiting time additional events are lost. @@ -323,9 +322,10 @@ const { fallback = 'animate' } = Astro.props as Props; }); addEventListener('popstate', (ev) => { - if (!transitionEnabledOnThisPage()) { + if (!transitionEnabledOnThisPage() && ev.state) { // The current page doesn't haven't View Transitions, // respect that with a full page reload + // -- but only for transition managed by us (ev.state is set) location.reload(); return; } diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/half-baked.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/half-baked.astro new file mode 100644 index 000000000..40298d125 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/half-baked.astro @@ -0,0 +1,24 @@ +--- +import { ViewTransitions } from 'astro:transitions'; + +// For the test fixture, we import the script but we don't use the component +// While this seems to be some strange mistake, +// it might be realistic, e.g. in a configurable CommenHead component + +interface Props { + transitions?: string; +} +const { transitions } = Astro.props; +--- + + + Half-Baked + {transitions && } + + +
+

Half Baked

+ hash target +
+ + diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/three.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/three.astro index 676e8b61b..eddc049a8 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/three.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/three.astro @@ -6,6 +6,9 @@

Page 3

go to 2 +
+ hash target +

Long paragraph

diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 69fa7c55f..c681bfea0 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -112,6 +112,40 @@ test.describe('View Transitions', () => { ).toEqual(2); }); + test('Moving within a page without ViewTransitions does not trigger a full page navigation', async ({ + page, + astro, + }) => { + const loads = []; + page.addListener('load', async (p) => { + loads.push(p.title()); + }); + // Go to page 1 + await page.goto(astro.resolveUrl('/one')); + let p = page.locator('#one'); + await expect(p, 'should have content').toHaveText('Page 1'); + + // Go to page 3 which does *not* have ViewTransitions enabled + await page.click('#click-three'); + p = page.locator('#three'); + await expect(p, 'should have content').toHaveText('Page 3'); + + // click a hash link to navigate further down the page + await page.click('#click-hash'); + // still on page 3 + p = page.locator('#three'); + await expect(p, 'should have content').toHaveText('Page 3'); + + // check that we are further down the page + const Y = await page.evaluate(() => window.scrollY); + expect(Y, 'The target is further down the page').toBeGreaterThan(0); + + expect( + loads.length, + 'There should be only 1 page load. The original, but no additional loads for the hash change' + ).toEqual(1); + }); + test('Moving from a page without ViewTransitions w/ back button', async ({ page, astro }) => { const loads = []; page.addListener('load', (p) => { @@ -332,4 +366,34 @@ test.describe('View Transitions', () => { await expect(loads.length, 'There should only be 1 page load').toEqual(1); }); + + test('Importing ViewTransitions w/o using the component must not mess with history', async ({ + page, + astro, + }) => { + const loads = []; + page.addListener('load', async (p) => { + loads.push(p); + }); + // Go to the half bakeed page + await page.goto(astro.resolveUrl('/half-baked')); + let p = page.locator('#half-baked'); + await expect(p, 'should have content').toHaveText('Half Baked'); + + // click a hash link to navigate further down the page + await page.click('#click-hash'); + // still on page + p = page.locator('#half-baked'); + await expect(p, 'should have content').toHaveText('Half Baked'); + + // go back within same page without reloading + await page.goBack(); + p = page.locator('#half-baked'); + await expect(p, 'should have content').toHaveText('Half Baked'); + + expect( + loads.length, + 'There should be only 1 page load. No additional loads for going back on same page' + ).toEqual(1); + }); });