Skip to content

Commit

Permalink
fix(astro): newer navigation aborts existing one (#10900)
Browse files Browse the repository at this point in the history
* Detection and cancelation of previous navigations and view transitions

* typos and wording

* typos and wording

* add test for animation cancelation

* second round

* final touches

* final final touches

* Clear the most recent navigation after view transition finished
  • Loading branch information
martrapp committed Apr 30, 2024
1 parent 5e545e8 commit 36bb3b6
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 64 deletions.
5 changes: 5 additions & 0 deletions .changeset/curly-spoons-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Detects overlapping navigation and view transitions and automatically aborts all but the most recent one.
29 changes: 29 additions & 0 deletions packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
import { ViewTransitions } from 'astro:transitions';
---
<html>
<head>
<ViewTransitions />
</head>
<body>
<h1>Abort</h1>
</body>
</html>
<script>
import {navigate } from 'astro:transitions/client';

document.addEventListener('astro:before-preparation', (e) => {
const originalLoader = e.loader;
e.loader = async () => {
const result = await originalLoader();
if (e.to.href.endsWith("/two")) {
// delay loading of /two
await new Promise((resolve) => setTimeout(resolve, 1100));
}
}
});
// starts later, but is faster and overtakes the slower navigation
setTimeout(()=>navigate("/one"), 400);
// starts now, but is to slow to keep its lead
navigate("/two");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
import { ViewTransitions, fade } from 'astro:transitions';
---
<html transition:animate="none">
<head>
<ViewTransitions />
</head>
<body>
<h1 transition:name="h1" transition:animate={fade({duration:10000})}>Abort</h1>
</body>
</html>

<style is:global>::view-transition-group(h1){animation:none}</style>
<script>
import {navigate } from 'astro:transitions/client';

setTimeout(()=>{
[...document.getAnimations()].forEach((a) => a.addEventListener('cancel',
(e) => console.log("[test]",e.type, a.animationName)));
console.log("[test] navigate to /one")
navigate("/one");
}, 1000);
console.log('[test] navigate to "."')
navigate("/abort2");
</script>
66 changes: 50 additions & 16 deletions packages/astro/e2e/view-transitions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1426,21 +1426,55 @@ test.describe('View Transitions', () => {
'all animations for transition:names should have been found'
).toEqual(0);
});
});

test('transition:persist persists selection', async ({ page, astro }) => {
let text = '';
page.on('console', (msg) => {
text = msg.text();
});
await page.goto(astro.resolveUrl('/persist-1'));
await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
// go to page 2
await page.press('input[name="name"]', 'Enter');
await expect(page.locator('#two'), 'should have content').toHaveText('Persist 2');
expect(text).toBe('true some cool text 5 9');

await page.goBack();
await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
expect(text).toBe('true true');
test('transition:persist persists selection', async ({ page, astro }) => {
let text = '';
page.on('console', (msg) => {
text = msg.text();
});
await page.goto(astro.resolveUrl('/persist-1'));
await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
// go to page 2
await page.press('input[name="name"]', 'Enter');
await expect(page.locator('#two'), 'should have content').toHaveText('Persist 2');
expect(text).toBe('true some cool text 5 9');

await page.goBack();
await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
expect(text).toBe('true true');
});

test('Navigation should be interruptible', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/abort'));
// implemented in /abort:
// clicks on slow loading page two
// after short delay clicks on fast loading page one
// even after some delay /two should not show up
await new Promise((resolve) => setTimeout(resolve, 2000)); // wait is part of the test
let p = page.locator('#one');
await expect(p, 'should have content').toHaveText('Page 1');
});

test('animation get canceled when view transition is interrupted', async ({ page, astro }) => {
let lines = [];
page.on('console', (msg) => {
msg.text().startsWith('[test]') && lines.push(msg.text());
});
await page.goto(astro.resolveUrl('/abort2'));
// implemented in /abort2:
// Navigate to self with a 10 second animation
// shortly after starting that, change your mind an navigate to /one
// check that animations got canceled
await new Promise((resolve) => setTimeout(resolve, 1000)); // wait is part of the test
let p = page.locator('#one');
await expect(p, 'should have content').toHaveText('Page 1');

// This test would be more important for a browser without native view transitions
// as those do not have automatic cancelation of transitions.
// For simulated view transitions, the last line would be missing as enter and exit animations
// don't run in parallel.
expect(lines.join('\n')).toBe(
'[test] navigate to "."\n[test] navigate to /one\n[test] cancel astroFadeOut\n[test] cancel astroFadeIn'
);
});
});
17 changes: 13 additions & 4 deletions packages/astro/src/transitions/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class BeforeEvent extends Event {
readonly sourceElement: Element | undefined;
readonly info: any;
newDocument: Document;
signal: AbortSignal;

constructor(
type: string,
Expand All @@ -35,7 +36,8 @@ class BeforeEvent extends Event {
navigationType: NavigationTypeString,
sourceElement: Element | undefined,
info: any,
newDocument: Document
newDocument: Document,
signal: AbortSignal
) {
super(type, eventInitDict);
this.from = from;
Expand All @@ -45,6 +47,7 @@ class BeforeEvent extends Event {
this.sourceElement = sourceElement;
this.info = info;
this.newDocument = newDocument;
this.signal = signal;

Object.defineProperties(this, {
from: { enumerable: true },
Expand All @@ -54,6 +57,7 @@ class BeforeEvent extends Event {
sourceElement: { enumerable: true },
info: { enumerable: true },
newDocument: { enumerable: true, writable: true },
signal: { enumerable: true },
});
}
}
Expand All @@ -76,6 +80,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent {
sourceElement: Element | undefined,
info: any,
newDocument: Document,
signal: AbortSignal,
formData: FormData | undefined,
loader: (event: TransitionBeforePreparationEvent) => Promise<void>
) {
Expand All @@ -88,7 +93,8 @@ export class TransitionBeforePreparationEvent extends BeforeEvent {
navigationType,
sourceElement,
info,
newDocument
newDocument,
signal
);
this.formData = formData;
this.loader = loader.bind(this, this);
Expand Down Expand Up @@ -124,7 +130,8 @@ export class TransitionBeforeSwapEvent extends BeforeEvent {
afterPreparation.navigationType,
afterPreparation.sourceElement,
afterPreparation.info,
afterPreparation.newDocument
afterPreparation.newDocument,
afterPreparation.signal
);
this.direction = afterPreparation.direction;
this.viewTransition = viewTransition;
Expand All @@ -145,6 +152,7 @@ export async function doPreparation(
navigationType: NavigationTypeString,
sourceElement: Element | undefined,
info: any,
signal: AbortSignal,
formData: FormData | undefined,
defaultLoader: (event: TransitionBeforePreparationEvent) => Promise<void>
) {
Expand All @@ -156,6 +164,7 @@ export async function doPreparation(
sourceElement,
info,
window.document,
signal,
formData,
defaultLoader
);
Expand All @@ -172,7 +181,7 @@ export async function doPreparation(
return event;
}

export async function doSwap(
export function doSwap(
afterPreparation: BeforeEvent,
viewTransition: ViewTransition,
defaultSwap: (event: TransitionBeforeSwapEvent) => void
Expand Down
Loading

0 comments on commit 36bb3b6

Please sign in to comment.