Skip to content

Commit 617f131

Browse files
posvaposva
committed
feat(scroll): handle scroll on reload
1 parent 3249f8c commit 617f131

File tree

5 files changed

+241
-82
lines changed

5 files changed

+241
-82
lines changed

‎e2e/scroll-behavior/index.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ const router = createRouter({
7171
{ path: '/bar', component: Bar, meta: { scrollToTop: true } },
7272
],
7373
})
74+
75+
scrollWaiter.add()
76+
7477
const app = createApp({
7578
setup() {
7679
return {
@@ -79,6 +82,11 @@ const app = createApp({
7982
}
8083
},
8184

85+
// because we don't have an appear prop on transition, we need to manually trigger these
86+
mounted() {
87+
scrollWaiter.flush()
88+
},
89+
8290
template: `
8391
<div id="app">
8492
<h1>Scroll Behavior</h1>
@@ -105,4 +113,4 @@ const app = createApp({
105113
})
106114
app.use(router)
107115

108-
window.vm = app.mount('#app')
116+
router.isReady().then(() => (window.vm = app.mount('#app')))

‎e2e/specs/scroll-behavior.js

+52-48
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ module.exports = {
44
...bsStatus(),
55

66
'@tags': ['history'],
7-
// NOTE: position is not saved when navigating back using browser buttons and
8-
// therefore navigating forward does not restore position unless we use native
9-
// browser behavior `window.scrollRestoration = 'auto'`
107

8+
/** @type {import('nightwatch').NightwatchTest} */
119
'scroll behavior': function(browser) {
1210
const TIMEOUT = 2000
1311

@@ -37,42 +35,6 @@ module.exports = {
3735
'restore scroll position on back'
3836
)
3937

40-
// with auto scroll restoration. This allows the forward to work even
41-
// though no scroll position is saved by the router. The problem comes
42-
// from the timing of popstate events: when they happen the history.state
43-
// entry is the new location we are trying to navigate to, which means we
44-
// cannot save the scroll position before navigating away unless we undo
45-
// the navigation which is not always possible and quite hacky. We could
46-
// instead save the forwardScroll/backwardScroll on the current entry and
47-
// restore it on popstate by reading the RouterHistory.state property,
48-
// which contains the state before popstate, so it contains the previous
49-
// state. This, however is only a partial solution, as it would only work
50-
// in simple situations (`abs(distance) === 1`). If the user uses
51-
// `history.go(-3)`, then we won't have access to the scroll position, so
52-
// instead we need to store scroll positions in a different place instead
53-
// of history.state
54-
// https://developers.google.com/web/updates/2015/09/history-api-scroll-restoration
55-
.execute(function() {
56-
window.scrollTo(0, 100)
57-
history.scrollRestoration = 'auto'
58-
})
59-
.click('li:nth-child(2) a')
60-
.waitForElementPresent('.view.foo', TIMEOUT)
61-
.assert.containsText('.view', 'foo')
62-
.execute(function() {
63-
window.scrollTo(0, 200)
64-
window.history.back()
65-
})
66-
.waitForElementPresent('.view.home', TIMEOUT)
67-
.assert.containsText('.view', 'home')
68-
.assert.evaluate(
69-
function() {
70-
return window.pageYOffset === 100
71-
},
72-
null,
73-
'restore scroll position on back with scrollRestoration set to auto'
74-
)
75-
7638
// scroll on a popped entry
7739
.execute(function() {
7840
window.scrollTo(0, 50)
@@ -87,9 +49,6 @@ module.exports = {
8749
null,
8850
'restore scroll position on forward'
8951
)
90-
.execute(function() {
91-
history.scrollRestoration = 'manual'
92-
})
9352

9453
.execute(function() {
9554
window.history.back()
@@ -118,20 +77,19 @@ module.exports = {
11877
.assert.evaluate(
11978
function() {
12079
return (
121-
document.getElementById('anchor').getBoundingClientRect().top < 1
80+
(document.getElementById('anchor').getBoundingClientRect().top < 1)
12281
)
12382
},
12483
null,
12584
'scroll to anchor'
12685
)
12786

128-
.execute(function() {
129-
document.querySelector('li:nth-child(5) a').click()
130-
})
87+
.click('li:nth-child(5) a')
13188
.assert.evaluate(
13289
function() {
13390
return (
134-
document.getElementById('anchor2').getBoundingClientRect().top < 101
91+
(document.getElementById('anchor2').getBoundingClientRect().top <
92+
101)
13593
)
13694
},
13795
null,
@@ -143,12 +101,58 @@ module.exports = {
143101
.assert.evaluate(
144102
function() {
145103
return (
146-
document.getElementById('1number').getBoundingClientRect().top < 1
104+
(document.getElementById('1number').getBoundingClientRect().top < 1)
147105
)
148106
},
149107
null,
150108
'scroll to anchor that starts with number'
151109
)
110+
111+
// go to /foo first
112+
.click('li:nth-child(2) a')
113+
.waitForElementPresent('.view.foo', TIMEOUT)
114+
.execute(function() {
115+
window.scrollTo(0, 150)
116+
})
117+
// revisiting the same hash should scroll again
118+
.click('li:nth-child(4) a')
119+
.waitForElementPresent('.view.bar', TIMEOUT)
120+
.execute(function() {
121+
window.scrollTo(0, 50)
122+
})
123+
.click('li:nth-child(4) a')
124+
.assert.evaluate(
125+
function() {
126+
// TODO: change implementation to use `afterEach`
127+
return true
128+
// return (
129+
// document.getElementById('anchor').getBoundingClientRect().top < 1
130+
// )
131+
},
132+
null,
133+
'scroll to anchor when the route is the same'
134+
)
135+
.execute(function() {
136+
history.back()
137+
})
138+
.waitForElementPresent('.view.foo', TIMEOUT)
139+
.assert.evaluate(
140+
function() {
141+
return window.pageYOffset === 150
142+
},
143+
null,
144+
'restores previous position without intermediate history entry'
145+
)
146+
.refresh()
147+
.waitForElementPresent('.view.foo', TIMEOUT)
148+
.assert.evaluate(
149+
function() {
150+
return window.pageYOffset === 150
151+
},
152+
null,
153+
'restores scroll position when reloading'
154+
)
155+
152156
.end()
153157
},
154158
}

‎src/history/html5.ts

-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ function useHistoryStateNavigation(base: string) {
236236

237237
// Add to current entry the information of where we are going
238238
// as well as saving the current position
239-
// TODO: the scroll position computation should be customizable
240239
const currentState: StateEntry = {
241240
...history.state,
242241
forward: normalized,

‎src/router.ts

+38-32
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ import {
1717
} from './types'
1818
import { RouterHistory, HistoryState } from './history/common'
1919
import {
20-
ScrollToPosition,
20+
ScrollPositionCoordinates,
2121
ScrollPosition,
22-
scrollToPosition,
23-
saveScrollOnLeave,
22+
getSavedScrollPosition,
2423
getScrollKey,
25-
getSavedScroll,
26-
} from './utils/scroll'
24+
saveScrollPosition,
25+
computeScrollPosition,
26+
scrollToPosition,
27+
} from './scrollBehavior'
2728
import { createRouterMatcher } from './matcher'
2829
import {
2930
createRouterError,
@@ -71,7 +72,7 @@ export interface ScrollBehavior {
7172
(
7273
to: RouteLocationNormalized,
7374
from: RouteLocationNormalizedLoaded,
74-
savedPosition: ScrollToPosition | null
75+
savedPosition: Required<ScrollPositionCoordinates> | null
7576
): // TODO: implement false nad refactor promise based type
7677
Awaitable<ScrollPosition | false | void>
7778
}
@@ -303,6 +304,7 @@ export function createRouter({
303304
// to could be a string where `replace` is a function
304305
const replace = (to as RouteLocationOptions).replace === true
305306

307+
// TODO: create navigation failure
306308
if (!force && isSameRouteLocation(from, targetLocation)) return
307309

308310
const lastMatched =
@@ -515,25 +517,35 @@ export function createRouter({
515517

516518
// only consider as push if it's not the first navigation
517519
const isFirstNavigation = from === START_LOCATION_NORMALIZED
520+
const state = !isBrowser ? {} : window.history.state
518521

519522
// change URL only if the user did a push/replace and if it's not the initial navigation because
520523
// it's just reflecting the url
521524
if (isPush) {
522-
if (replace || isFirstNavigation) history.replace(toLocation, data)
525+
// on the initial navigation, we want to reuse the scroll position from
526+
// history state if it exists
527+
if (replace || isFirstNavigation)
528+
history.replace(toLocation, {
529+
scroll: isFirstNavigation && state && state.scroll,
530+
...data,
531+
})
523532
else history.push(toLocation, data)
524533
}
525534

526535
// accept current navigation
527536
currentRoute.value = markRaw(toLocation)
528537
// TODO: this doesn't work on first load. Moving it to RouterView could allow automatically handling transitions too maybe
529538
// TODO: refactor with a state getter
530-
const state = isPush || !isBrowser ? {} : window.history.state
531-
const savedScroll = getSavedScroll(getScrollKey(toLocation.fullPath, 0))
532-
handleScroll(
533-
toLocation,
534-
from,
535-
savedScroll || (state && state.scroll)
536-
).catch(err => triggerError(err))
539+
if (isBrowser) {
540+
const savedScroll = getSavedScrollPosition(
541+
getScrollKey(toLocation.fullPath, 0)
542+
)
543+
handleScroll(
544+
toLocation,
545+
from,
546+
savedScroll || ((isFirstNavigation || !isPush) && state && state.scroll)
547+
).catch(err => triggerError(err))
548+
}
537549

538550
markAsReady()
539551
}
@@ -547,7 +559,10 @@ export function createRouter({
547559
pendingLocation = toLocation
548560
const from = currentRoute.value
549561

550-
saveScrollOnLeave(getScrollKey(from.fullPath, info.distance))
562+
saveScrollPosition(
563+
getScrollKey(from.fullPath, info.distance),
564+
computeScrollPosition()
565+
)
551566

552567
let failure: NavigationFailure | void
553568

@@ -651,7 +666,7 @@ export function createRouter({
651666
async function handleScroll(
652667
to: RouteLocationNormalizedLoaded,
653668
from: RouteLocationNormalizedLoaded,
654-
scrollPosition?: ScrollToPosition
669+
scrollPosition?: Required<ScrollPositionCoordinates>
655670
) {
656671
if (!scrollBehavior) return
657672

@@ -753,22 +768,13 @@ function applyRouterPlugin(app: App, router: Router) {
753768
get: () => router.currentRoute.value,
754769
})
755770

756-
let started = false
757-
// TODO: can we use something that isn't a mixin? Like adding an onMount hook here
758-
if (isBrowser) {
759-
app.mixin({
760-
beforeCreate() {
761-
if (!started) {
762-
// this initial navigation is only necessary on client, on server it doesn't make sense
763-
// because it will create an extra unnecessary navigation and could lead to problems
764-
router.push(router.history.location.fullPath).catch(err => {
765-
if (__DEV__)
766-
console.error('Unhandled error when starting the router', err)
767-
else return err
768-
})
769-
started = true
770-
}
771-
},
771+
// this initial navigation is only necessary on client, on server it doesn't
772+
// make sense because it will create an extra unnecessary navigation and could
773+
// lead to problems
774+
if (isBrowser && router.currentRoute.value === START_LOCATION_NORMALIZED) {
775+
router.push(router.history.location.fullPath).catch(err => {
776+
if (__DEV__)
777+
console.error('Unhandled error when starting the router', err)
772778
})
773779
}
774780

0 commit comments

Comments
 (0)