Fix flickering during view transitions (#8772)

* Fix for #8711 * more descriptive changeset * chores, while we are at it ...
withastro · Oct 9, 2023 · c24f70d · c24f70d
1 parent 33d0be5
commit c24f70d
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 2 deletions.
diff --git a/.changeset/seven-seas-hide.md b/.changeset/seven-seas-hide.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix flickering during view transitions
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
@@ -0,0 +1,34 @@
+---
+import { ViewTransitions } from 'astro:transitions';
+---
+<html>
+	<head>
+		<ViewTransitions/>
+	</head>
+	<body>
+		<p>Local transitions</p>
+		<slot/>
+		<script>
+			document.addEventListener("astro:after-swap", () => {
+				document.querySelector("p").addEventListener("transitionstart", () => {
+					console.info("transitionstart");
+				});
+				document.documentElement.setAttribute("class", "blue");
+			});
+			document.dispatchEvent(new Event("astro:after-swap"));
+		</script>
+	</body>
+	<style>
+		p {
+			transition: background-color 1s;
+		}
+		p {
+			background-color: #0ee;
+			color: red;
+		}
+		.blue p {
+			background-color: #ee0;
+			color: blue;
+		}
+	</style>
+</html>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/listener-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/listener-one.astro
@@ -0,0 +1,6 @@
+---
+import ListenerLayout from '../components/listener-layout.astro';
+---
+<ListenerLayout>
+	<a id="totwo" href="/listener-two">Go to listener two</a>
+</ListenerLayout>
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/listener-two.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/listener-two.astro
@@ -0,0 +1,6 @@
+---
+import ListenerLayout from '../components/listener-layout.astro';
+---
+<ListenerLayout>
+	<a id="toone" href="/listener-one">Go to listener one</a>
+</ListenerLayout>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -230,6 +230,28 @@ test.describe('View Transitions', () => {
 		await expect(h, 'imported CSS updated').toHaveCSS('background-color', 'rgba(0, 0, 0, 0)');
 	});
 
+	test('No page rendering during swap()', async ({ page, astro }) => {
+		let transitions = 0;
+		page.on('console', (msg) => {
+			if (msg.type() === 'info' && msg.text() === 'transitionstart') ++transitions;
+		});
+
+		// 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).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(1);
+	});
+
 	test('click hash links does not do navigation', async ({ page, astro }) => {
 		// Go to page 1
 		await page.goto(astro.resolveUrl('/one'));

diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
@@ -146,18 +146,24 @@ function isInfinite(animation: Animation) {
 
 const updateHistoryAndScrollPosition = (toLocation: URL, replace: boolean, intraPage: boolean) => {
 	const fresh = !samePage(toLocation);
+	let scrolledToTop = false;
 	if (toLocation.href !== location.href) {
 		if (replace) {
 			history.replaceState({ ...history.state }, '', toLocation.href);
 		} else {
 			history.replaceState({ ...history.state, intraPage }, '');
-			history.pushState({ index: ++currentHistoryIndex, scrollX, scrollY }, '', toLocation.href);
+			history.pushState(
+				{ index: ++currentHistoryIndex, scrollX: 0, scrollY: 0 },
+				'',
+				toLocation.href
+			);
 		}
 		// now we are on the new page for non-history navigations!
 		// (with history navigation page change happens before popstate is fired)
 		// freshly loaded pages start from the top
 		if (fresh) {
 			scrollTo({ left: 0, top: 0, behavior: 'instant' });
+			scrolledToTop = true;
 		}
 	}
 	if (toLocation.hash) {
@@ -166,7 +172,9 @@ const updateHistoryAndScrollPosition = (toLocation: URL, replace: boolean, intra
 		// that won't reload the page but instead scroll to the fragment
 		location.href = toLocation.href;
 	} else {
-		scrollTo({ left: 0, top: 0, behavior: 'instant' });
+		if (!scrolledToTop) {
+			scrollTo({ left: 0, top: 0, behavior: 'instant' });
+		}
 	}
 };