Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scroll restoration overhaul #3873

Closed
wants to merge 65 commits into from
Closed
Show file tree
Hide file tree
Changes from 55 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
1e7decd
push
PH4NTOMiki Feb 12, 2022
735dc1c
.
PH4NTOMiki Feb 12, 2022
9b02c51
.
PH4NTOMiki Feb 12, 2022
e30615e
.
PH4NTOMiki Feb 12, 2022
432f415
test2
PH4NTOMiki Feb 12, 2022
1efd2a7
.
PH4NTOMiki Feb 12, 2022
1d56889
trying option B
PH4NTOMiki Feb 12, 2022
d35ada5
option B without event listener removed
PH4NTOMiki Feb 12, 2022
5691f46
.
PH4NTOMiki Feb 12, 2022
1f6604d
.
PH4NTOMiki Feb 12, 2022
f9ce80f
wrong attempt
PH4NTOMiki Feb 13, 2022
55a6049
.
PH4NTOMiki Feb 13, 2022
36e81e2
scroll restoration overhaul
PH4NTOMiki Feb 13, 2022
b955f03
.
PH4NTOMiki Feb 13, 2022
d5fe979
test issues fixed
PH4NTOMiki Feb 13, 2022
b5a3a20
.
PH4NTOMiki Feb 13, 2022
2f9adeb
.
PH4NTOMiki Feb 13, 2022
b2b906f
uncomment previously failing test
benmccann Feb 13, 2022
3a25400
.
PH4NTOMiki Feb 13, 2022
605210f
Revert "uncomment previously failing test"
PH4NTOMiki Feb 13, 2022
14e5e53
type
PH4NTOMiki Feb 13, 2022
375c134
.
PH4NTOMiki Feb 13, 2022
effbdbf
.
PH4NTOMiki Feb 13, 2022
a2e9ad9
try to fix test
PH4NTOMiki Feb 13, 2022
4255912
rerun test
PH4NTOMiki Feb 13, 2022
75175c5
fix
PH4NTOMiki Feb 13, 2022
4822069
trying with setting stores manually
PH4NTOMiki Feb 13, 2022
8e27aec
.
PH4NTOMiki Feb 13, 2022
159567e
testing
PH4NTOMiki Feb 13, 2022
708170f
lint
PH4NTOMiki Feb 13, 2022
4b2df43
clg this
PH4NTOMiki Feb 13, 2022
af167ef
.
PH4NTOMiki Feb 13, 2022
5c3fc29
.
PH4NTOMiki Feb 13, 2022
2da7e9e
.
PH4NTOMiki Feb 13, 2022
37735ab
try
PH4NTOMiki Feb 13, 2022
e5986b0
.
PH4NTOMiki Feb 13, 2022
ec41da6
working
PH4NTOMiki Feb 13, 2022
cf7f9b5
empty commit to trigger test
PH4NTOMiki Feb 14, 2022
fa62c5f
trying to change in_view
PH4NTOMiki Feb 14, 2022
f39b81a
test passed, trying again, empty commit
PH4NTOMiki Feb 14, 2022
366fe31
trying another in_view function
PH4NTOMiki Feb 14, 2022
e6c830d
clg rect
PH4NTOMiki Feb 14, 2022
bad40a4
.
PH4NTOMiki Feb 14, 2022
3d7d389
.
PH4NTOMiki Feb 14, 2022
dd0a1f6
trying the old trick
PH4NTOMiki Feb 14, 2022
44d2636
empty commit, retest
PH4NTOMiki Feb 14, 2022
d9373a3
Update packages/kit/src/runtime/client/router.js
PH4NTOMiki Feb 14, 2022
f986ce7
revert tests and test utils, they should be changed in separate PR
PH4NTOMiki Feb 15, 2022
61606c2
revert test
PH4NTOMiki Feb 15, 2022
39f34fd
cleanup and should fix ctrl shift t issue
PH4NTOMiki Feb 15, 2022
c88c9b4
fix
PH4NTOMiki Feb 15, 2022
d8527f3
using sessionStorage instead of deprecated window.performance method
PH4NTOMiki Feb 15, 2022
646dd69
Revert "revert test"
benmccann Feb 15, 2022
c12c44e
Update packages/kit/src/runtime/client/router.js
benmccann Feb 15, 2022
296a652
Update packages/kit/src/runtime/client/router.js
PH4NTOMiki Feb 15, 2022
b141009
Update packages/kit/src/runtime/client/router.js
benmccann Feb 15, 2022
037d24f
go back to previous version of test
benmccann Feb 15, 2022
f8f3db1
trying fix for FireFox
PH4NTOMiki Feb 15, 2022
427cc50
lint FF fix
PH4NTOMiki Feb 15, 2022
4fc8c3d
different FF fix
PH4NTOMiki Feb 15, 2022
60dad67
fix2
PH4NTOMiki Feb 15, 2022
ee51709
FF fix
PH4NTOMiki Feb 15, 2022
8d44831
removed unneeded code
PH4NTOMiki Feb 15, 2022
4fac58e
Merge branch 'master' into remove-scroll-listener
Rich-Harris Feb 15, 2022
0b9e769
Merge branch 'master' into remove-scroll-listener
Rich-Harris Feb 15, 2022
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
5 changes: 5 additions & 0 deletions .changeset/shy-taxis-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

scroll restoration overhaul
86 changes: 56 additions & 30 deletions packages/kit/src/runtime/client/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,26 @@ export class Router {
// make it possible to reset focus
document.body.setAttribute('tabindex', '-1');

history.scrollRestoration = 'manual';
PH4NTOMiki marked this conversation as resolved.
Show resolved Hide resolved

// keeping track of the history index in order to prevent popstate navigation events if needed
this.current_history_index = history.state?.['sveltekit:index'] ?? 0;
/** @type {number} */
this.current_history_index = Number(
sessionStorage.getItem('sveltekit:index') ?? history.state?.['sveltekit:index'] ?? 0
);

if (this.current_history_index === 0) {
if (history.state?.['sveltekit:index'] === undefined) {
// create initial history entry, so we can return here
history.replaceState({ ...history.state, 'sveltekit:index': 0 }, '', location.href);
history.replaceState(
{ ...history.state, 'sveltekit:index': this.current_history_index },
'',
location.href
);
} else if (sessionStorage.getItem('sveltekit:left_page') === 'true') {
PH4NTOMiki marked this conversation as resolved.
Show resolved Hide resolved
// detects return to the page
sessionStorage.removeItem('sveltekit:left_page');
const scroll = this.#get_scroll_position(this.current_history_index);
if (scroll) scrollTo(scroll.x, scroll.y);
}

this.callbacks = {
Expand All @@ -72,11 +86,28 @@ export class Router {
};
}

init_listeners() {
if ('scrollRestoration' in history) {
history.scrollRestoration = 'manual';
#save_scroll_position() {
sessionStorage.setItem(
`sveltekit:scroll:${this.current_history_index}`,
JSON.stringify(scroll_state())
);
}

/**
* @param {number} index
* @returns {{x: number, y: number} | null}
*/
#get_scroll_position(index) {
if (index == undefined) return null;
const scroll = sessionStorage.getItem(`sveltekit:scroll:${index}`);
if (scroll) {
return JSON.parse(scroll);
} else {
return null;
}
}

init_listeners() {
// Adopted from Nuxt.js
// Reset scrollRestoration to auto when leaving page, allowing page reload
// and back-navigation from other pages to use the browser to restore the
Expand All @@ -96,34 +127,18 @@ export class Router {
e.preventDefault();
e.returnValue = '';
} else {
history.scrollRestoration = 'auto';
this.#save_scroll_position();
sessionStorage.setItem('sveltekit:index', String(this.current_history_index));
sessionStorage.setItem('sveltekit:left_page', 'true');
}
});

// Setting scrollRestoration to manual again when returning to this page.
// We can probably safely delete this because constructor executes when returned to the page, but we need to test this to be sure, letting it stay for now.
addEventListener('load', () => {
history.scrollRestoration = 'manual';
});

// There's no API to capture the scroll location right before the user
// hits the back/forward button, so we listen for scroll events

/** @type {NodeJS.Timeout} */
let scroll_timer;
addEventListener('scroll', () => {
clearTimeout(scroll_timer);
scroll_timer = setTimeout(() => {
// Store the scroll location in the history
// This will persist even if we navigate away from the site and come back
const new_state = {
...(history.state || {}),
'sveltekit:scroll': scroll_state()
};
history.replaceState(new_state, document.title, window.location.href);
// iOS scroll event intervals happen between 30-150ms, sometimes around 200ms
}, 200);
});

/** @param {Event} event */
const trigger_prefetch = (event) => {
const a = find_anchor(event);
Expand Down Expand Up @@ -190,9 +205,17 @@ export class Router {
// Removing the hash does a full page navigation in the browser, so make sure a hash is present
const [base, hash] = url.href.split('#');
if (hash !== undefined && base === location.href.split('#')[0]) {
// Call `pushState` to add url to history so going back works.
// Also make a delay, otherwise the browser default behaviour would not kick in
setTimeout(() => history.pushState({}, '', url.href));
this.#save_scroll_position();

// Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state
// Timeout is being used to delay replaceState till next tick
setTimeout(() => {
history.replaceState(
{ ...(history.state || {}), 'sveltekit:index': ++this.current_history_index },
benmccann marked this conversation as resolved.
Show resolved Hide resolved
'',
location.href
);
});
const info = this.parse(url);
if (info) {
return this.renderer.update(info, [], false);
Expand Down Expand Up @@ -220,9 +243,11 @@ export class Router {
// with history.go, which means we end up back here, hence this check
if (event.state['sveltekit:index'] === this.current_history_index) return;

this.#save_scroll_position();

this._navigate({
url: new URL(location.href),
scroll: event.state['sveltekit:scroll'],
scroll: this.#get_scroll_position(event.state['sveltekit:index']),
keepfocus: false,
chain: [],
details: null,
Expand Down Expand Up @@ -385,6 +410,7 @@ export class Router {
});
}

this.#save_scroll_position();
accepted();

if (!this.navigating) {
Expand Down
8 changes: 3 additions & 5 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,9 @@ test.describe('Scrolling', () => {
expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2');
const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY));
expect(scrollY).toEqual(originalScrollY);
// TODO: fix this. it is failing due to duplicate history entries
// https://github.com/sveltejs/kit/issues/3636
// await page.goBack();
// expect(page.url()).toBe(baseURL + '/anchor');
// expect(scrollY).toEqual(0);
await page.goBack();
expect(page.url()).toBe(baseURL + '/anchor');
expect(scrollY).toEqual(0);
});

test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({
Expand Down
1 change: 0 additions & 1 deletion packages/kit/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ export const test = base.extend({
setTimeout(() => {
reject(new Error('Timed out'));
}, 5000);

addEventListener('sveltekit:start', () => {
fulfil();
});
Expand Down