Skip to content

Commit

Permalink
Chores: Tidying up the view transition router (#8889)
Browse files Browse the repository at this point in the history 
* Chores: simplifications and documentation

* improvements after review comments

* make linter happy

* More leniency with different browser behavior
  • Loading branch information
@martrapp @natemoo-re
martrapp authored and natemoo-re committed Nov 22, 2023
1 parent b4b7a42 commit 57c2a61
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 79 deletions.
11 changes: 3 additions & 8 deletions packages/astro/e2e/fixtures/view-transitions/src/components/listener-layout.astro
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
44 changes: 29 additions & 15 deletions packages/astro/e2e/view-transitions.test.js
View file
Original file line number Diff line number Diff line change
@@ -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/' });

Expand Down Expand Up @@ -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 }) => {
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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);
Expand Down
110 changes: 54 additions & 56 deletions packages/astro/src/transitions/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 }, '');
}
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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') {
Expand All @@ -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();
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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 ?? {});
Expand Down Expand Up @@ -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 = '';
}
}

Expand All @@ -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);
}
});
}
Expand Down

0 comments on commit 57c2a61

Please sign in to comment.