Skip to content

Commit 4de7d87

Browse files
danrobertsdanroberts
authored
Merge pull request #393 from gadget-inc/droberts/GGT-1931-no-scroll-on-replace-nav-on-page
fix(navigation): don't scroll to hash on replace navigation within page
2 parents b6fd7a9 + fd2dc86 commit 4de7d87

File tree

7 files changed

+220
-9
lines changed

7 files changed

+220
-9
lines changed

‎packages/fastify-renderer/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "fastify-renderer",
3-
"version": "0.3.0",
3+
"version": "0.3.1",
44
"description": "Simple, high performance client side app renderer for Fastify.",
55
"exports": {
66
".": {

‎packages/fastify-renderer/src/client/react/Root.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useEffect, useState } from 'react'
2-
import { Route, Router, Switch, useLocation } from 'wouter'
2+
import { Route, Router, Switch, useLocation, useRouter } from 'wouter'
33
import { usePromise } from './fetcher'
4-
import { useNavigationDetails, useTransitionLocation } from './locationHook'
4+
import { shouldScrollToHash, useNavigationDetails, useTransitionLocation } from './locationHook'
55
import { matcher } from './matcher'
66

77
export interface LayoutProps {
@@ -47,6 +47,7 @@ export function Root<BootProps>(props: {
4747
<Route path={route} key={route}>
4848
{(params) => {
4949
const [location] = useLocation()
50+
const router = useRouter()
5051
const backendPath = location.split('#')[0] // remove current anchor for fetching data from the server side
5152

5253
const payload = usePromise<{ props: Record<string, any> }>(props.basePath + backendPath, async () => {
@@ -65,9 +66,11 @@ export function Root<BootProps>(props: {
6566
}
6667
})
6768

68-
// navigate to the anchor in the url after rendering
69+
// Navigate to the anchor in the url after rendering, unless we're using replaceState and
70+
// the destination page and previous page have the same base route (i.e. before '#')
71+
// We would do this for example to update the url to the correct anchor as the user scrolls.
6972
useEffect(() => {
70-
if (window.location.hash) {
73+
if (window.location.hash && shouldScrollToHash(router.navigationHistory)) {
7174
document.getElementById(window.location.hash.slice(1))?.scrollIntoView()
7275
}
7376
}, [location])

‎packages/fastify-renderer/src/client/react/locationHook.ts

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { unstable_useTransition as useTransition, useCallback, useEffect, useRef, useState } from 'react'
2-
import { useLocation } from 'wouter'
2+
import { NavigationHistory, useLocation, useRouter } from 'wouter'
33

44
/**
55
* History API docs @see https://developer.mozilla.org/en-US/docs/Web/API/History
@@ -20,6 +20,16 @@ export const useTransitionLocation = ({ base = '' } = {}) => {
2020
const [path, update] = useState(() => currentPathname(base)) // @see https://reactjs.org/docs/hooks-reference.html#lazy-initial-state
2121
const prevLocation = useRef(path + location.search + location.hash)
2222
const [startTransition, isPending] = useTransition()
23+
const router = useRouter()
24+
useEffect(() => {
25+
if (!router.navigationHistory)
26+
router.navigationHistory = {
27+
current: {
28+
path,
29+
replace: false,
30+
},
31+
}
32+
}, [])
2333

2434
useEffect(() => {
2535
// this function checks if the location has been changed since the
@@ -31,9 +41,13 @@ export const useTransitionLocation = ({ base = '' } = {}) => {
3141

3242
if (prevLocation.current !== destination) {
3343
prevLocation.current = destination
34-
startTransition(() => {
44+
if (shouldScrollToHash(router.navigationHistory)) {
45+
startTransition(() => {
46+
update(destination)
47+
})
48+
} else {
3549
update(destination)
36-
})
50+
}
3751
}
3852
}
3953

@@ -61,11 +75,23 @@ export const useTransitionLocation = ({ base = '' } = {}) => {
6175
return
6276
}
6377

78+
const path = base + to
79+
80+
if (!router.navigationHistory) router.navigationHistory = {}
81+
if (router.navigationHistory?.current) {
82+
router.navigationHistory.previous = { ...router.navigationHistory.current }
83+
}
84+
85+
router.navigationHistory.current = {
86+
path,
87+
replace,
88+
}
89+
6490
history[replace ? eventReplaceState : eventPushState](
6591
null,
6692
'',
6793
// handle nested routers and absolute paths
68-
base + to
94+
path
6995
)
7096
},
7197
[base]
@@ -105,3 +131,16 @@ export const useNavigationDetails = (): [boolean, string] => {
105131

106132
const currentPathname = (base, path = location.pathname + location.search + location.hash) =>
107133
!path.toLowerCase().indexOf(base.toLowerCase()) ? path.slice(base.length) || '/' : '~' + path
134+
135+
export const navigatingOnSamePage = (history?: NavigationHistory): boolean => {
136+
const { current, previous } = history || {}
137+
138+
if (!history) return false
139+
if (!current || !previous) return false
140+
141+
return current.path.split('#')[0] == previous.path.split('#')[0]
142+
}
143+
144+
export const shouldScrollToHash = (history?: NavigationHistory): boolean => {
145+
return !(navigatingOnSamePage(history) && history?.current?.replace)
146+
}

‎packages/fastify-renderer/src/client/react/wouter-extension.d.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import 'wouter'
2+
3+
declare module 'wouter' {
4+
export interface RouterProps {
5+
navigationHistory?: NavigationHistory
6+
}
7+
8+
export interface NavigationHistory {
9+
current?: NavigationHistoryItem
10+
previous?: NavigationHistoryItem
11+
}
12+
13+
export interface NavigationHistoryItem {
14+
path: string
15+
replace: boolean
16+
}
17+
}

‎packages/test-apps/simple-react/NavigationHistoryTest.tsx

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import React from 'react'
2+
import { Link, useLocation } from 'wouter'
3+
4+
const NavigationHistoryTest = () => {
5+
const [path, navigate] = useLocation()
6+
7+
return (
8+
<>
9+
<h1>Navigation Test</h1>
10+
<p>Leaving this page will set the navigation details on the window for inspection in the tests</p>
11+
<br />
12+
<Link href="/navigation-history-test#section">
13+
<a id="section-link">Go to the content</a>
14+
</Link>
15+
<br />
16+
<Link href="/">
17+
<a id="home-link">Home</a>
18+
</Link>
19+
<br />
20+
<button
21+
id="section-link-replace"
22+
onClick={() => {
23+
navigate('/navigation-history-test#section', { replace: true })
24+
}}
25+
>
26+
Update url without scrolling
27+
</button>
28+
<br />
29+
<br />
30+
<br />
31+
<br />
32+
<br />
33+
<br />
34+
<br />
35+
<br />
36+
<br />
37+
<br />
38+
<br />
39+
<br />
40+
<br />
41+
<br />
42+
<br />
43+
<br />
44+
<br />
45+
<br />
46+
<br />
47+
<br />
48+
<br />
49+
<br />
50+
<br />
51+
<br />
52+
<br />
53+
<br />
54+
<br />
55+
<br />
56+
<br />
57+
<br />
58+
<br />
59+
<br />
60+
<br />
61+
<br />
62+
<br />
63+
<br />
64+
<br />
65+
<br />
66+
<br />
67+
<br />
68+
<br />
69+
<br />
70+
<br />
71+
<br />
72+
<br />
73+
<br />
74+
<br />
75+
<br />
76+
<br />
77+
<br />
78+
<br />
79+
<br />
80+
<br />
81+
<br />
82+
<br />
83+
<br />
84+
<br />
85+
<br />
86+
<br />
87+
<br />
88+
<br />
89+
<h2 id="section">Another Section</h2>
90+
<p>
91+
Lorem ipsum dolor sit amet consectetur adipisicing elit. Fuga earum maiores, excepturi aspernatur perspiciatis
92+
doloribus suscipit voluptates ipsam nam in nostrum vel obcaecati cum illum ex quasi quo est at.
93+
</p>
94+
</>
95+
)
96+
}
97+
98+
export default NavigationHistoryTest

‎packages/test-apps/simple-react/server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ export const server = async () => {
6464
return {}
6565
})
6666

67+
server.get('/navigation-history-test', { render: require.resolve('./NavigationHistoryTest') }, async (_request) => {
68+
return {}
69+
})
70+
6771
await server.register(async (instance) => {
6872
instance.setRenderConfig({ document: CustomDocumentTemplate })
6973

‎packages/test-apps/simple-react/test/navigation-history.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { Page } from 'playwright-chromium'
2+
import { newTestPage, reactReady, rootURL } from '../../helpers'
3+
4+
describe('navigation details', () => {
5+
let page: Page
6+
7+
beforeEach(async () => {
8+
page = await newTestPage()
9+
await page.goto(`${rootURL}/navigation-history-test`)
10+
await reactReady(page)
11+
})
12+
13+
test('navigating to an anchor will scroll down to the anchor', async () => {
14+
const visibleBeforeClick = await isIntersectingViewport(page, '#section')
15+
expect(visibleBeforeClick).toBe(false)
16+
17+
await page.click('#section-link')
18+
19+
const visibleAfterClick = await isIntersectingViewport(page, '#section')
20+
expect(visibleAfterClick).toBe(true)
21+
})
22+
23+
test('navigating to an anchor that is on the same page via replace: true will not scroll to the anchor', async () => {
24+
const visibleBeforeClick = await isIntersectingViewport(page, '#section')
25+
expect(visibleBeforeClick).toBe(false)
26+
27+
await page.click('#section-link-replace')
28+
29+
const visibleAfterClick = await isIntersectingViewport(page, '#section')
30+
expect(visibleAfterClick).toBe(false)
31+
})
32+
})
33+
34+
const isIntersectingViewport = (page: Page, selector: string): Promise<boolean> => {
35+
return page.$eval(selector, async (element) => {
36+
const visibleRatio: number = await new Promise((resolve) => {
37+
const observer = new IntersectionObserver((entries) => {
38+
resolve(entries[0].intersectionRatio)
39+
observer.disconnect()
40+
})
41+
observer.observe(element)
42+
// Firefox doesn't call IntersectionObserver callback unless
43+
// there are rafs.
44+
requestAnimationFrame(() => {
45+
/**/
46+
})
47+
})
48+
return visibleRatio > 0
49+
})
50+
}

0 commit comments

Comments
 (0)