From 35c0984230c9cebf70153c50c4ff8eeebb3f42ef Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Wed, 8 May 2024 15:08:55 +0200 Subject: [PATCH] proposal for better support for custom swap functions (#10908) * proposal for custom swap function support * correct 'Fix typos' * Suggestion * update tests to use new pattern * test order of chaining * tidy up * renaming 'swapper: CustomSwapper' to 'swapSteps: SwapSteps' * after first review * fix linter error * updated test to not use swap-steps * removed another swapSteps reference --------- Co-authored-by: Luiz Ferraz --- .../view-transitions/src/pages/chaining.astro | 49 ++++++ .../src/pages/half-baked.astro | 2 +- .../src/pages/keep-style-one.astro | 28 ++++ .../src/pages/keep-theme-one.astro | 27 ++++ .../view-transitions/src/pages/keep-two.astro | 9 ++ .../src/pages/replace-main-one.astro | 34 +++++ packages/astro/e2e/view-transitions.test.js | 69 +++++++++ packages/astro/src/transitions/events.ts | 9 +- packages/astro/src/transitions/router.ts | 142 ++--------------- .../astro/src/transitions/swap-functions.ts | 143 ++++++++++++++++++ 10 files changed, 373 insertions(+), 139 deletions(-) create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/chaining.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/keep-style-one.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/keep-theme-one.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/keep-two.astro create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/replace-main-one.astro create mode 100644 packages/astro/src/transitions/swap-functions.ts diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/chaining.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/chaining.astro new file mode 100644 index 0000000000..e96b45b8d5 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/chaining.astro @@ -0,0 +1,49 @@ +--- +import Layout from "../components/Layout.astro" +--- + +

Chaining

+ click +
+ + 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 index 76e36dbdfa..bbb9ef1053 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/half-baked.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/half-baked.astro @@ -3,7 +3,7 @@ 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 CommentHead component +// it might be realistic, e.g. in a configurable CommonHead component interface Props { transitions?: string; diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-style-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-style-one.astro new file mode 100644 index 0000000000..413a404d76 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-style-one.astro @@ -0,0 +1,28 @@ +--- +import Layout from '../components/Layout.astro'; +--- + +

Keep Style

+ go to next page +
+ diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-theme-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-theme-one.astro new file mode 100644 index 0000000000..a4c942d587 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-theme-one.astro @@ -0,0 +1,27 @@ +--- +import Layout from '../components/Layout.astro'; +--- + +

Keep Theme

+ go to next page +
+ diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-two.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-two.astro new file mode 100644 index 0000000000..60751e82ee --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/keep-two.astro @@ -0,0 +1,9 @@ +--- +import Layout from '../components/Layout.astro'; +--- + +
+

Keep 2

+

This is the main section from page 2

+
+
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/replace-main-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/replace-main-one.astro new file mode 100644 index 0000000000..1709661e8a --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/replace-main-one.astro @@ -0,0 +1,34 @@ +--- +import Layout from '../components/Layout.astro'; +--- + +
+

Replace Main Section

+

This is the main section

+
+ go to next page +
+ diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 7a94fdc0ca..d17202e619 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1444,6 +1444,75 @@ test.describe('View Transitions', () => { expect(text).toBe('true true'); }); + + test('it should be easy to define a data-theme preserving swap function', async ({ + page, + astro, + }) => { + await page.goto(astro.resolveUrl('/keep-theme-one')); + await expect(page.locator('#name'), 'should have content').toHaveText('Keep Theme'); + await page.$eval(':root', (element) => element.setAttribute('data-theme', 'purple')); + + await page.click('#click'); + await expect(page.locator('#name'), 'should have content').toHaveText('Keep 2'); + + const attributeValue = await page.$eval( + ':root', + (element, attributeName) => element.getAttribute(attributeName), + 'data-theme' + ); + expect(attributeValue).toBe('purple'); + }); + + test('it should be easy to define a swap function that preserves a dynamically generated style sheet', async ({ + page, + astro, + }) => { + await page.goto(astro.resolveUrl('/keep-style-one')); + await expect(page.locator('#name'), 'should have content').toHaveText('Keep Style'); + await page.evaluate(() => { + const style = document.createElement('style'); + style.textContent = 'body { background-color: purple; }'; + document.head.insertAdjacentElement('afterbegin', style); + }); + + await page.click('#click'); + await expect(page.locator('#name'), 'should have content').toHaveText('Keep 2'); + + const styleElement = await page.$('head > style'); + const styleContent = await page.evaluate((style) => style.innerHTML, styleElement); + expect(styleContent).toBe('body { background-color: purple; }'); + }); + + test('it should be easy to define a swap function that only swaps the main area', async ({ + page, + astro, + }) => { + await page.goto(astro.resolveUrl('/replace-main-one')); + await expect(page.locator('#name'), 'should have content').toHaveText('Replace Main Section'); + + await page.click('#click'); + // name inside
should have changed + await expect(page.locator('#name'), 'should have content').toHaveText('Keep 2'); + + // link outside
should still be there + const link = await page.$('#click'); + expect(link).toBeTruthy(); + }); + + test('chaining should execute in the expected order', async ({ page, astro }) => { + let lines = []; + page.on('console', (msg) => { + msg.text().startsWith('[test]') && lines.push(msg.text().slice('[test]'.length + 1)); + }); + + await page.goto(astro.resolveUrl('/chaining')); + await expect(page.locator('#name'), 'should have content').toHaveText('Chaining'); + await page.click('#click'); + await expect(page.locator('#one'), 'should have content').toHaveText('Page 1'); + expect(lines.join('..')).toBe('5..4..3..2..1..0'); + }); + test('Navigation should be interruptible', async ({ page, astro }) => { await page.goto(astro.resolveUrl('/abort')); // implemented in /abort: diff --git a/packages/astro/src/transitions/events.ts b/packages/astro/src/transitions/events.ts index a35fd72cb6..9665c5821a 100644 --- a/packages/astro/src/transitions/events.ts +++ b/packages/astro/src/transitions/events.ts @@ -1,3 +1,4 @@ +import { swap } from './swap-functions.js'; import { updateScrollPosition } from './router.js'; import type { Direction, NavigationTypeString } from './types.js'; @@ -118,8 +119,7 @@ export class TransitionBeforeSwapEvent extends BeforeEvent { constructor( afterPreparation: BeforeEvent, - viewTransition: ViewTransition, - swap: (event: TransitionBeforeSwapEvent) => void + viewTransition: ViewTransition ) { super( TRANSITION_BEFORE_SWAP, @@ -135,7 +135,7 @@ export class TransitionBeforeSwapEvent extends BeforeEvent { ); this.direction = afterPreparation.direction; this.viewTransition = viewTransition; - this.swap = swap.bind(this, this); + this.swap = () => swap(this.newDocument); Object.defineProperties(this, { direction: { enumerable: true }, @@ -184,9 +184,8 @@ export async function doPreparation( export function doSwap( afterPreparation: BeforeEvent, viewTransition: ViewTransition, - defaultSwap: (event: TransitionBeforeSwapEvent) => void ) { - const event = new TransitionBeforeSwapEvent(afterPreparation, viewTransition, defaultSwap); + const event = new TransitionBeforeSwapEvent(afterPreparation, viewTransition); document.dispatchEvent(event); event.swap(); return event; diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 9ce9d4560f..ba3556d868 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -1,5 +1,13 @@ import type { TransitionBeforePreparationEvent, TransitionBeforeSwapEvent } from './events.js'; import { TRANSITION_AFTER_SWAP, doPreparation, doSwap } from './events.js'; +import { + deselectScripts, + restoreFocus, + saveFocus, + swapBodyElement, + swapHeadElements, + swapRootAttributes, +} from './swap-functions.js'; import type { Direction, Fallback, Options } from './types.js'; type State = { @@ -264,138 +272,6 @@ async function updateDOM( historyState?: State, fallback?: Fallback ) { - // 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, newDoc: Document): Element | null => { - const id = el.getAttribute(PERSIST_ATTR); - const newEl = id && newDoc.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); - if (newEl) { - return newEl; - } - if (el.matches('link[rel=stylesheet]')) { - const href = el.getAttribute('href'); - return newDoc.head.querySelector(`link[rel=stylesheet][href="${href}"]`); - } - return null; - }; - - type SavedFocus = { - activeElement: HTMLElement | null; - start?: number | null; - end?: number | null; - }; - - const saveFocus = (): SavedFocus => { - const activeElement = document.activeElement as HTMLElement; - // The element that currently has the focus is part of a DOM tree - // that will survive the transition to the new document. - // Save the element and the cursor position - if (activeElement?.closest(`[${PERSIST_ATTR}]`)) { - if ( - activeElement instanceof HTMLInputElement || - activeElement instanceof HTMLTextAreaElement - ) { - const start = activeElement.selectionStart; - const end = activeElement.selectionEnd; - return { activeElement, start, end }; - } - return { activeElement }; - } else { - return { activeElement: null }; - } - }; - - const restoreFocus = ({ activeElement, start, end }: SavedFocus) => { - if (activeElement) { - activeElement.focus(); - if ( - activeElement instanceof HTMLInputElement || - activeElement instanceof HTMLTextAreaElement - ) { - if (typeof start === 'number') activeElement.selectionStart = start; - if (typeof end === 'number') activeElement.selectionEnd = end; - } - } - }; - - const shouldCopyProps = (el: HTMLElement): boolean => { - const persistProps = el.dataset.astroTransitionPersistProps; - return persistProps == null || persistProps === 'false'; - }; - - const defaultSwap = (beforeSwapEvent: TransitionBeforeSwapEvent) => { - // swap attributes of the html element - // - delete all attributes from the current document - // - insert all attributes from doc - // - reinsert all original attributes that are named 'data-astro-*' - const html = document.documentElement; - const astroAttributes = [...html.attributes].filter( - ({ name }) => (html.removeAttribute(name), name.startsWith('data-astro-')) - ); - [...beforeSwapEvent.newDocument.documentElement.attributes, ...astroAttributes].forEach( - ({ name, value }) => html.setAttribute(name, value) - ); - - // Replace scripts in both the head and body. - for (const s1 of document.scripts) { - for (const s2 of beforeSwapEvent.newDocument.scripts) { - if ( - // Check if the script should be rerun regardless of it being the same - !s2.hasAttribute('data-astro-rerun') && - // Inline - ((!s1.src && s1.textContent === s2.textContent) || - // External - (s1.src && s1.type === s2.type && s1.src === s2.src)) - ) { - // the old script is in the new document and doesn't have the rerun attribute - // we mark it as executed to prevent re-execution - s2.dataset.astroExec = ''; - break; - } - } - } - - // Swap head - for (const el of Array.from(document.head.children)) { - const newEl = persistedHeadElement(el as HTMLElement, beforeSwapEvent.newDocument); - // If the element exists in the document already, remove it - // from the new document and leave the current node alone - if (newEl) { - newEl.remove(); - } else { - // Otherwise remove the element in the head. It doesn't exist in the new page. - el.remove(); - } - } - - // Everything left in the new head is new, append it all. - document.head.append(...beforeSwapEvent.newDocument.head.children); - - // Persist elements in the existing body - const oldBody = document.body; - - const savedFocus = saveFocus(); - - // this will reset scroll Position - document.body.replaceWith(beforeSwapEvent.newDocument.body); - - for (const el of oldBody.querySelectorAll(`[${PERSIST_ATTR}]`)) { - const id = el.getAttribute(PERSIST_ATTR); - const newEl = document.querySelector(`[${PERSIST_ATTR}="${id}"]`); - if (newEl) { - // The element exists in the new page, replace it with the element - // from the old page so that state is preserved. - newEl.replaceWith(el); - // For islands, copy over the props to allow them to re-render - if (newEl.localName === 'astro-island' && shouldCopyProps(el as HTMLElement)) { - el.setAttribute('ssr', ''); - el.setAttribute('props', newEl.getAttribute('props')!); - } - } - } - restoreFocus(savedFocus); - }; async function animate(phase: string) { function isInfinite(animation: Animation) { @@ -430,7 +306,7 @@ async function updateDOM( } const pageTitleForBrowserHistory = document.title; // document.title will be overridden by swap() - const swapEvent = doSwap(preparationEvent, currentTransition.viewTransition!, defaultSwap); + const swapEvent = doSwap(preparationEvent, currentTransition.viewTransition!); moveToLocation(swapEvent.to, swapEvent.from, options, pageTitleForBrowserHistory, historyState); triggerEvent(TRANSITION_AFTER_SWAP); diff --git a/packages/astro/src/transitions/swap-functions.ts b/packages/astro/src/transitions/swap-functions.ts new file mode 100644 index 0000000000..97c637ccd0 --- /dev/null +++ b/packages/astro/src/transitions/swap-functions.ts @@ -0,0 +1,143 @@ +export type SavedFocus = { + activeElement: HTMLElement | null; + start?: number | null; + end?: number | null; +}; + +const PERSIST_ATTR = 'data-astro-transition-persist'; + +/* + * Mark new scripts that should not execute + */ +export function deselectScripts(doc: Document) { + for (const s1 of document.scripts) { + for (const s2 of doc.scripts) { + if ( + // Check if the script should be rerun regardless of it being the same + !s2.hasAttribute('data-astro-rerun') && + // Inline + ((!s1.src && s1.textContent === s2.textContent) || + // External + (s1.src && s1.type === s2.type && s1.src === s2.src)) + ) { + // the old script is in the new document and doesn't have the rerun attribute + // we mark it as executed to prevent re-execution + s2.dataset.astroExec = ''; + break; + } + } + } +} + +/* + * swap attributes of the html element + * delete all attributes from the current document + * insert all attributes from doc + * reinsert all original attributes that are named 'data-astro-*' + */ +export function swapRootAttributes(doc: Document) { + const html = document.documentElement; + const astroAttributes = [...html.attributes].filter( + ({ name }) => (html.removeAttribute(name), name.startsWith('data-astro-')) + ); + [...doc.documentElement.attributes, ...astroAttributes].forEach(({ name, value }) => + html.setAttribute(name, value) + ); +} + +/* + * make the old head look like the new one + */ +export function swapHeadElements(doc: Document) { + for (const el of Array.from(document.head.children)) { + const newEl = persistedHeadElement(el as HTMLElement, doc); + // If the element exists in the document already, remove it + // from the new document and leave the current node alone + if (newEl) { + newEl.remove(); + } else { + // Otherwise remove the element in the head. It doesn't exist in the new page. + el.remove(); + } + } + + // Everything left in the new head is new, append it all. + document.head.append(...doc.head.children); +} + +export function swapBodyElement(newElement: Element, oldElement: Element) { + // this will reset scroll Position + oldElement.replaceWith(newElement); + + for (const el of oldElement.querySelectorAll(`[${PERSIST_ATTR}]`)) { + const id = el.getAttribute(PERSIST_ATTR); + const newEl = newElement.querySelector(`[${PERSIST_ATTR}="${id}"]`); + if (newEl) { + // The element exists in the new page, replace it with the element + // from the old page so that state is preserved. + newEl.replaceWith(el); + // For islands, copy over the props to allow them to re-render + if (newEl.localName === 'astro-island' && shouldCopyProps(el as HTMLElement)) { + el.setAttribute('ssr', ''); + el.setAttribute('props', newEl.getAttribute('props')!); + } + } + } +} + +export const saveFocus = (): (() => void) => { + const activeElement = document.activeElement as HTMLElement; + // The element that currently has the focus is part of a DOM tree + // that will survive the transition to the new document. + // Save the element and the cursor position + if (activeElement?.closest(`[${PERSIST_ATTR}]`)) { + if (activeElement instanceof HTMLInputElement || activeElement instanceof HTMLTextAreaElement) { + const start = activeElement.selectionStart; + const end = activeElement.selectionEnd; + return () => restoreFocus({ activeElement, start, end }); + } + return () => restoreFocus({ activeElement }); + } else { + return () => restoreFocus({ activeElement: null }); + } +}; + +export const restoreFocus = ({ activeElement, start, end }: SavedFocus) => { + if (activeElement) { + activeElement.focus(); + if (activeElement instanceof HTMLInputElement || activeElement instanceof HTMLTextAreaElement) { + if (typeof start === 'number') activeElement.selectionStart = start; + if (typeof end === 'number') activeElement.selectionEnd = end; + } + } +}; + +// 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, newDoc: Document): Element | null => { + const id = el.getAttribute(PERSIST_ATTR); + const newEl = id && newDoc.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); + if (newEl) { + return newEl; + } + if (el.matches('link[rel=stylesheet]')) { + const href = el.getAttribute('href'); + return newDoc.head.querySelector(`link[rel=stylesheet][href="${href}"]`); + } + return null; +}; + +const shouldCopyProps = (el: HTMLElement): boolean => { + const persistProps = el.dataset.astroTransitionPersistProps; + return persistProps == null || persistProps === 'false'; +}; + +export const swap = (doc:Document) => { + deselectScripts(doc); + swapRootAttributes(doc); + swapHeadElements(doc); + const restoreFocusFunction = saveFocus(); + swapBodyElement(doc.body, document.body) + restoreFocusFunction(); +}