Skip to content

Commit

Permalink
allow disableScrollHandling() to be called in afterNavigate (#4948)
Browse files Browse the repository at this point in the history
* allow disableScrollHandling to be called in afterNavigate - fixes #3220

* fix tests

* remove .only

* format
  • Loading branch information
Rich-Harris committed May 23, 2022
1 parent b2a8c5c commit 77a1a73
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/polite-files-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

allow disableScrollHandling to be called in afterNavigate
38 changes: 22 additions & 16 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ export function create_client({ target, session, base, trailing_slash }) {
* @param {string[]} redirect_chain
* @param {boolean} no_cache
* @param {{hash?: string, scroll: { x: number, y: number } | null, keepfocus: boolean, details: { replaceState: boolean, state: any } | null}} [opts]
* @param {() => void} [callback]
*/
async function update(url, redirect_chain, no_cache, opts) {
async function update(url, redirect_chain, no_cache, opts, callback) {
const intent = get_navigation_intent(url);

const current_token = (token = {});
Expand Down Expand Up @@ -360,7 +361,6 @@ export function create_client({ target, session, base, trailing_slash }) {
load_cache.promise = null;
load_cache.id = null;
autoscroll = true;
updating = false;

if (navigation_result.props.page) {
page = navigation_result.props.page;
Expand All @@ -369,7 +369,9 @@ export function create_client({ target, session, base, trailing_slash }) {
const leaf_node = navigation_result.state.branch[navigation_result.state.branch.length - 1];
router_enabled = leaf_node?.module.router !== false;

return true;
if (callback) callback();

updating = false;
}

/** @param {import('./types').NavigationResult} result */
Expand All @@ -387,12 +389,12 @@ export function create_client({ target, session, base, trailing_slash }) {
hydrate: true
});

started = true;

if (router_enabled) {
const navigation = { from: null, to: new URL(location.href) };
callbacks.after_navigate.forEach((fn) => fn(navigation));
}

started = true;
}

/**
Expand Down Expand Up @@ -978,18 +980,22 @@ export function create_client({ target, session, base, trailing_slash }) {
});
}

const completed = await update(normalized, redirect_chain, false, {
scroll,
keepfocus,
details
});

if (completed) {
const navigation = { from, to: normalized };
callbacks.after_navigate.forEach((fn) => fn(navigation));
await update(
normalized,
redirect_chain,
false,
{
scroll,
keepfocus,
details
},
() => {
const navigation = { from, to: normalized };
callbacks.after_navigate.forEach((fn) => fn(navigation));

stores.navigating.set(null);
}
stores.navigating.set(null);
}
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
import { afterNavigate, disableScrollHandling } from '$app/navigation';
afterNavigate(() => {
disableScrollHandling();
document.getElementById('abcde')?.scrollIntoView();
});
</script>
<div style="height: 180vh; background-color: hotpink;">They (don't) see me...</div>
<div style="height: 180vh; background-color: peru;">
<p id="go-to-element">The browser scrolls to me</p>
</div>
<p id="abcde" style="height: 180vh; background-color: hotpink;">I take precedence</p>
<div />
<a href="/anchor-with-manual-scroll/anchor-afternavigate?x=y#go-to-element">reload me</a>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<h1>Welcome to a test project</h1>
<a href="/anchor-with-manual-scroll/anchor#go-to-element">Anchor demo</a>
<a href="/anchor-with-manual-scroll/anchor-onmount#go-to-element">Anchor demo</a>

<style>
:global(body) {
Expand Down
16 changes: 14 additions & 2 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,19 @@ test.describe('Scrolling', () => {
page,
in_view
}) => {
await page.goto('/anchor-with-manual-scroll/anchor#go-to-element');
await page.goto('/anchor-with-manual-scroll/anchor-onmount#go-to-element');
expect(await in_view('#abcde')).toBe(true);
});

test('url-supplied anchor is ignored with afterNavigate() scrolling on direct page load', async ({
page,
in_view,
clicknav
}) => {
await page.goto('/anchor-with-manual-scroll/anchor-afternavigate#go-to-element');
expect(await in_view('#abcde')).toBe(true);

await clicknav('[href="/anchor-with-manual-scroll/anchor-afternavigate?x=y#go-to-element"]');
expect(await in_view('#abcde')).toBe(true);
});

Expand All @@ -270,7 +282,7 @@ test.describe('Scrolling', () => {
in_view
}) => {
await page.goto('/anchor-with-manual-scroll');
await clicknav('[href="/anchor-with-manual-scroll/anchor#go-to-element"]');
await clicknav('[href="/anchor-with-manual-scroll/anchor-onmount#go-to-element"]');
if (javaScriptEnabled) expect(await in_view('#abcde')).toBe(true);
else expect(await in_view('#go-to-element')).toBe(true);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/types/ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ declare module '$app/env' {
*/
declare module '$app/navigation' {
/**
* If called when the page is being updated following a navigation (in `onMount` or an action, for example), this disables SvelteKit's built-in scroll handling.
* If called when the page is being updated following a navigation (in `onMount` or `afterNavigate` or an action, for example), this disables SvelteKit's built-in scroll handling.
* This is generally discouraged, since it breaks user expectations.
*/
export function disableScrollHandling(): void;
Expand Down

0 comments on commit 77a1a73

Please sign in to comment.