Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 60 additions & 14 deletions core/src/components/router/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { NavigationHookResult } from '../route/route-interface';

import { ROUTER_INTENT_BACK, ROUTER_INTENT_FORWARD, ROUTER_INTENT_NONE } from './utils/constants';
import { printRedirects, printRoutes } from './utils/debug';
import { readNavState, waitUntilNavNode, writeNavState } from './utils/dom';
import { readNavState, scrollToFragment, waitUntilNavNode, writeNavState } from './utils/dom';
import type { RouteChain, RouterDirection, RouterEventDetail } from './utils/interface';
import { findChainForIDs, findChainForSegments, findRouteRedirect } from './utils/matching';
import { readRedirects, readRoutes } from './utils/parser';
Expand All @@ -24,6 +24,7 @@ export class Router implements ComponentInterface {
private state = 0;
private lastState = 0;
private waitPromise?: Promise<void>;
private fragmentScrollToken = 0;

@Element() el!: HTMLElement;

Expand Down Expand Up @@ -67,11 +68,18 @@ export class Router implements ComponentInterface {
if (typeof canProceed === 'object') {
const { redirect } = canProceed;
const path = parsePath(redirect);
this.setSegments(path.segments, ROUTER_INTENT_NONE, path.queryString);
await this.writeNavStateRoot(path.segments, ROUTER_INTENT_NONE);
this.setSegments(path.segments, ROUTER_INTENT_NONE, path.queryString, path.fragment);
const result = await this.writeNavStateRoot(path.segments, ROUTER_INTENT_NONE);
if (result) {
this.maybeScrollToFragment();
}
}
} else {
await this.onRoutesChanged();
return;
}

const result = await this.onRoutesChanged();
if (result) {
this.maybeScrollToFragment();
}
}

Expand All @@ -93,7 +101,12 @@ export class Router implements ComponentInterface {
return false;
}
}
return this.writeNavStateRoot(segments, direction);
const result = await this.writeNavStateRoot(segments, direction);
if (result) {
this.maybeScrollToFragment();
}

return result;
}

@Listen('ionBackButton', { target: 'document' })
Expand Down Expand Up @@ -132,7 +145,7 @@ export class Router implements ComponentInterface {
const currentPath = this.previousPath ?? '/';
// Convert currentPath to an URL by pre-pending a protocol and a host to resolve the relative path.
const url = new URL(path, `https://host/${currentPath}`);
path = url.pathname + url.search;
path = url.pathname + url.search + url.hash;
}

let parsedPath = parsePath(path);
Expand All @@ -146,8 +159,13 @@ export class Router implements ComponentInterface {
}
}

this.setSegments(parsedPath.segments, direction, parsedPath.queryString);
return this.writeNavStateRoot(parsedPath.segments, direction, animation);
this.setSegments(parsedPath.segments, direction, parsedPath.queryString, parsedPath.fragment);
const result = await this.writeNavStateRoot(parsedPath.segments, direction, animation);
if (result) {
this.maybeScrollToFragment();
}

return result;
}

/** Go back to previous page in the window.history. */
Expand Down Expand Up @@ -188,7 +206,12 @@ export class Router implements ComponentInterface {
return false;
}

this.setSegments(segments, direction);
// navChanged is an outlet-driven URL sync. Only keep the fragment when
// the path is unchanged; on a real navigation it refers to an anchor on
// the page being left and would be stale.
const newPath = generatePath(segments);
const fragment = newPath === this.previousPath ? this.getFragment() : undefined;
this.setSegments(segments, direction, undefined, fragment);

await this.safeWriteNavState(outlet, chain, ROUTER_INTENT_NONE, segments, null, ids.length);
return true;
Expand Down Expand Up @@ -245,8 +268,8 @@ export class Router implements ComponentInterface {
let redirectFrom: string[] | null = null;

if (redirect) {
const { segments: toSegments, queryString } = redirect.to!;
this.setSegments(toSegments, direction, queryString);
const { segments: toSegments, queryString, fragment } = redirect.to!;
this.setSegments(toSegments, direction, queryString, fragment);
redirectFrom = redirect.from;
segments = toSegments;
}
Expand Down Expand Up @@ -361,15 +384,38 @@ export class Router implements ComponentInterface {
return changed;
}

private setSegments(segments: string[], direction: RouterDirection, queryString?: string) {
private setSegments(segments: string[], direction: RouterDirection, queryString?: string, fragment?: string) {
this.state++;
writeSegments(window.history, this.root, this.useHash, segments, direction, this.state, queryString);
// Every URL write invalidates any in-flight fragment scroll: a newer nav
// (with or without a fragment, successful or not) should always supersede.
this.fragmentScrollToken++;
writeSegments(window.history, this.root, this.useHash, segments, direction, this.state, queryString, fragment);
}

private getSegments(): string[] | null {
return readSegments(window.location, this.root, this.useHash);
}

private getFragment(): string | undefined {
// In hash mode the URL fragment trails a second `#` (e.g. `#/path#anchor`);
// parse the routing portion to extract it.
const raw = this.useHash ? parsePath(window.location.hash.slice(1)).fragment : window.location.hash.slice(1);
return raw ? raw : undefined;
}

/**
* Fires a best-effort scroll to the current URL fragment. The scroll bails
* if a newer `setSegments` advances `fragmentScrollToken` mid-flight.
*/
private maybeScrollToFragment() {
const fragment = this.getFragment();
if (!fragment) return;
const token = this.fragmentScrollToken;
// Fire-and-forget; the returned promise resolves only after the scroll
// animation completes, which the caller does not need to await.
scrollToFragment(fragment, () => token === this.fragmentScrollToken).catch(() => {});
}

private routeChangeEvent(toSegments: string[], redirectFromSegments: string[] | null): RouterEventDetail | null {
const from = this.previousPath;
const to = generatePath(toSegments);
Expand Down
9 changes: 8 additions & 1 deletion core/src/components/router/test/basic/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
<p><a href='#/two/second-page'>Go to page 2</a></p>
<p><a href='#/two/three/hola'>Go to page 3 (hola)</a></p>
<p><a href='#/two/three/something'>Go to page 3 (something)</a></p>

<p><ion-router-link id="link-with-fragment" href="/two/second-page#anchor">Page 2 with fragment</ion-router-link></p>
<p><ion-router-link id="link-with-query-and-fragment" href="/two/three/hola?flag=true#anchor">Page 3 with query and fragment</ion-router-link></p>
<div style="height: 2000px;">page-one spacer</div>
<h2 id="anchor">page-one anchor (must lose to the active page's anchor)</h2>
</ion-content>`;
}
}
Expand All @@ -42,6 +45,9 @@
<ion-content>
<p><a href='#/two/three/hola'>Go to page 3 (hola)</a></p>
<p><a href='#/two/three/hello'>Go to page 3 (hello)</a></p>
<div style="height: 2000px;">spacer</div>
<h2 id="anchor">Anchor target</h2>
<div style="height: 500px;">trailing spacer</div>
</ion-content>`;
}
}
Expand All @@ -60,6 +66,7 @@
<p><a href='#/two/second-page'>Go to page 2</a></p>
<p><a href='#/two/'>Go to page 1</a></p>
<ion-button id="btn-rel" href="./relative?param=1">Page 3 (relative)</ion-button>
<ion-button id="btn-rel-with-fragment" href="./relative#anchor">Page 3 (relative with fragment)</ion-button>
<ion-button id="btn-abs" href="/two/three/absolute">Page 3 (absolute)</ion-button>
</ion-content>`;
}
Expand Down
197 changes: 197 additions & 0 deletions core/src/components/router/test/basic/router.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
import { expect } from '@playwright/test';
import type { E2EPage } from '@utils/test/playwright';
import { configs, test } from '@utils/test/playwright';

/**
* Waits until `page-two`'s `ion-content` has scrolled past the fixture's 2000px
* spacer. The anchor target sits below the spacer, so a successful fragment
* scroll must move `scrollTop` well past it; a regression that scrolled by
* only a handful of pixels would fail this threshold.
*/
const waitForAnchorScrolled = (page: E2EPage) =>
page.waitForFunction(async () => {
const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
if (!content) return false;
const scrollEl = await content.getScrollElement();
return scrollEl.scrollTop > 1500;
});

/**
* This behavior does not vary across modes/directions.
*/
Expand All @@ -27,6 +42,188 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>

expect(page.url()).toContain('#/two/three/absolute');
});

test('should route when ion-router-link href contains a fragment', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
Comment thread
ShaneK marked this conversation as resolved.
});
const errors: string[] = [];
page.on('pageerror', (e) => errors.push(e.message));
page.on('console', (msg) => {
if (msg.type() === 'error') errors.push(msg.text());
});

await page.goto(`/src/components/router/test/basic#/two`, config);
await page.click('#link-with-fragment');

await expect(page.locator('page-two')).toBeVisible();
Comment thread
brandyscarney marked this conversation as resolved.
expect(page.url()).toContain('#/two/second-page#anchor');
expect(errors.filter((m) => m.includes('not part of the routing set'))).toEqual([]);
});

test('should route when ion-router-link href contains both query and fragment', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
await page.goto(`/src/components/router/test/basic#/two`, config);
await page.click('#link-with-query-and-fragment');

await expect(page.locator('page-three')).toBeVisible();
Comment thread
brandyscarney marked this conversation as resolved.
expect(page.url()).toContain('#/two/three/hola?flag=true#anchor');
});

test('should preserve the fragment when push() resolves a relative path', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
await page.goto(`/src/components/router/test/basic#/two/three/hola`, config);
await page.click('#btn-rel-with-fragment');

expect(page.url()).toContain('#/two/three/relative#anchor');
});

test('should scroll to the fragment target after navigating', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
await page.goto(`/src/components/router/test/basic#/two`, config);
await page.click('#link-with-fragment');

await expect(page.locator('page-two #anchor')).toBeVisible();
await waitForAnchorScrolled(page);
});

test('should scroll to the fragment target on initial deep-link load', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
// Land on the fixture without a fragment first so the test helper can
// attach its query params (it appends them after the hash, which would
// otherwise pollute the fragment). Once loaded we replaceState to a URL
// that includes the fragment, then reload to simulate a true cold open.
await page.goto(`/src/components/router/test/basic#/two`, config);
await page.evaluate(() => {
const { origin, pathname, search } = window.location;
window.history.replaceState({}, '', `${origin}${pathname}${search}#/two/second-page#anchor`);
});
await page.reload();

await expect(page.locator('page-two #anchor')).toBeVisible();
await waitForAnchorScrolled(page);
});

test('should scroll on deep-link load even when an inactive tab has hydrated', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
// Inactive `ion-tab` elements carry `.ion-page` but use `.tab-hidden`
// instead of `.ion-page-hidden`. The fixture's inline `tab-four` is one
// such sibling. Waiting for it to hydrate before reload makes the
// active-page lookup deterministic across runs.
await page.goto(`/src/components/router/test/basic#/two`, config);
await page.waitForFunction(() => !!document.querySelector('ion-tab[tab="tab-four"].hydrated'));
await page.evaluate(() => {
const { origin, pathname, search } = window.location;
window.history.replaceState({}, '', `${origin}${pathname}${search}#/two/second-page#anchor`);
});
await page.reload();
await page.waitForFunction(() => !!document.querySelector('ion-tab[tab="tab-four"].hydrated'));

await expect(page.locator('page-two #anchor')).toBeVisible();
await waitForAnchorScrolled(page);
});

test('should scope the fragment lookup to the active page', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
// page-one and page-two both expose `id="anchor"`. page-one is kept in
// the DOM as `.ion-page-hidden` after the push; a document-wide
// `getElementById` would return its anchor first. The router must scope
// the lookup to the active page so page-two's anchor wins.
await page.goto(`/src/components/router/test/basic#/two`, config);
await page.click('#link-with-fragment');

await expect(page.locator('page-two #anchor')).toBeVisible();
await waitForAnchorScrolled(page);

// page-one is still in the DOM but should not have been scrolled.
const pageOneScrollTop = await page.evaluate(async () => {
const content = document.querySelector('page-one ion-content') as HTMLIonContentElement | null;
if (!content) return 0;
const scrollEl = await content.getScrollElement();
return scrollEl.scrollTop;
});
expect(pageOneScrollTop).toBeLessThan(100);
});

test('should drop a stale fragment when navChanged fires for a different path', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
// Land on a URL with a fragment, then trigger a tab switch. The tab
// outlet emits `navChanged` for the new path; the fragment referred to
// an anchor on the previous page and must not survive the rewrite.
await page.goto(`/src/components/router/test/basic#/two/second-page#anchor`, config);
await expect(page.locator('page-two')).toBeVisible();

await page.click('#tab-button-tab-one');

await expect(page.locator('tab-one')).toBeVisible();
await page.waitForFunction(() => !window.location.hash.includes('#anchor'));
expect(page.url()).not.toContain('#anchor');
});

test('should cancel an in-flight fragment scroll when a newer navigation supersedes it', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
});
// Two rapid pushes: the first targets a fragment (begins polling +
// smooth scroll), the second arrives before the first lands and clears
// the fragment. The cancellation token must abort the first scroll so
// we end up at the top of the page, not parked at #anchor.
await page.goto(`/src/components/router/test/basic#/two`, config);
await expect(page.locator('page-one')).toBeVisible();

await page.evaluate(async () => {
const router = document.querySelector('ion-router') as HTMLIonRouterElement;
router.push('/two/second-page#anchor');
await router.push('/two/second-page');
});

await expect(page.locator('page-two')).toBeVisible();
// Wait for page-two's scrollTop to stabilise across two consecutive
// frames. A scroll triggered by the un-cancelled first push would
// still be animating when the assertion runs.
await page.waitForFunction(async () => {
const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
if (!content) return false;
const scrollEl = await content.getScrollElement();
const first = scrollEl.scrollTop;
await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve)));
return scrollEl.scrollTop === first;
});

const pageTwoScrollTop = await page.evaluate(async () => {
const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
if (!content) return -1;
const scrollEl = await content.getScrollElement();
return scrollEl.scrollTop;
});
expect(pageTwoScrollTop).toBeLessThan(100);
expect(page.url()).not.toContain('#anchor');
});
});

test.describe(title('router: tabs'), () => {
Expand Down
Loading
Loading