From f27c4e7e8de8a54b143002b83d0ee6d23e0f0da7 Mon Sep 17 00:00:00 2001 From: Daniel Roberts Date: Wed, 3 Aug 2022 14:41:47 -0400 Subject: [PATCH 1/2] fix(navigation): don't scroll to hash on replace navigation within page When we do a client side page navigation via wouter, if there is a hash anchor in the url, we scroll to that anchor to match the default browser behavior. This means that if we want to update the URL to match the user's current scroll location in the document, we end up triggering a second scroll. To avoid this, do not scroll to the anchor if the following conditions are met: - The navigation is happening on the same page: (i.e. from some/url#foo to some/url#bar) - The navigation was triggered by a history.replaceState() action --- packages/fastify-renderer/package.json | 2 +- .../src/client/react/Root.tsx | 11 ++++-- .../src/client/react/locationHook.ts | 38 +++++++++++++++++-- .../src/client/react/wouter-extension.d.ts | 17 +++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 packages/fastify-renderer/src/client/react/wouter-extension.d.ts diff --git a/packages/fastify-renderer/package.json b/packages/fastify-renderer/package.json index 31684c90..c10f40c8 100644 --- a/packages/fastify-renderer/package.json +++ b/packages/fastify-renderer/package.json @@ -1,6 +1,6 @@ { "name": "fastify-renderer", - "version": "0.3.0", + "version": "0.3.1", "description": "Simple, high performance client side app renderer for Fastify.", "exports": { ".": { diff --git a/packages/fastify-renderer/src/client/react/Root.tsx b/packages/fastify-renderer/src/client/react/Root.tsx index 8700c66c..db46ade3 100644 --- a/packages/fastify-renderer/src/client/react/Root.tsx +++ b/packages/fastify-renderer/src/client/react/Root.tsx @@ -1,7 +1,7 @@ import React, { useEffect, useState } from 'react' -import { Route, Router, Switch, useLocation } from 'wouter' +import { Route, Router, Switch, useLocation, useRouter } from 'wouter' import { usePromise } from './fetcher' -import { useNavigationDetails, useTransitionLocation } from './locationHook' +import { shouldScrollToHash, useNavigationDetails, useTransitionLocation } from './locationHook' import { matcher } from './matcher' export interface LayoutProps { @@ -47,6 +47,7 @@ export function Root(props: { {(params) => { const [location] = useLocation() + const router = useRouter() const backendPath = location.split('#')[0] // remove current anchor for fetching data from the server side const payload = usePromise<{ props: Record }>(props.basePath + backendPath, async () => { @@ -65,9 +66,11 @@ export function Root(props: { } }) - // navigate to the anchor in the url after rendering + // Navigate to the anchor in the url after rendering, unless we're using replaceState and + // the destination page and previous page have the same base route (i.e. before '#') + // We would do this for example to update the url to the correct anchor as the user scrolls. useEffect(() => { - if (window.location.hash) { + if (window.location.hash && shouldScrollToHash(router.navigationHistory)) { document.getElementById(window.location.hash.slice(1))?.scrollIntoView() } }, [location]) diff --git a/packages/fastify-renderer/src/client/react/locationHook.ts b/packages/fastify-renderer/src/client/react/locationHook.ts index 2d533440..269cf023 100644 --- a/packages/fastify-renderer/src/client/react/locationHook.ts +++ b/packages/fastify-renderer/src/client/react/locationHook.ts @@ -1,5 +1,5 @@ import { unstable_useTransition as useTransition, useCallback, useEffect, useRef, useState } from 'react' -import { useLocation } from 'wouter' +import { NavigationHistory, useLocation, useRouter } from 'wouter' /** * History API docs @see https://developer.mozilla.org/en-US/docs/Web/API/History @@ -20,6 +20,7 @@ export const useTransitionLocation = ({ base = '' } = {}) => { const [path, update] = useState(() => currentPathname(base)) // @see https://reactjs.org/docs/hooks-reference.html#lazy-initial-state const prevLocation = useRef(path + location.search + location.hash) const [startTransition, isPending] = useTransition() + const router = useRouter() useEffect(() => { // this function checks if the location has been changed since the @@ -31,9 +32,13 @@ export const useTransitionLocation = ({ base = '' } = {}) => { if (prevLocation.current !== destination) { prevLocation.current = destination - startTransition(() => { + if (shouldScrollToHash(router.navigationHistory)) { + startTransition(() => { + update(destination) + }) + } else { update(destination) - }) + } } } @@ -61,11 +66,23 @@ export const useTransitionLocation = ({ base = '' } = {}) => { return } + const path = base + to + + if (!router.navigationHistory) router.navigationHistory = {} + if (router.navigationHistory?.current) { + router.navigationHistory.previous = { ...router.navigationHistory.current } + } + + router.navigationHistory.current = { + path, + replace, + } + history[replace ? eventReplaceState : eventPushState]( null, '', // handle nested routers and absolute paths - base + to + path ) }, [base] @@ -105,3 +122,16 @@ export const useNavigationDetails = (): [boolean, string] => { const currentPathname = (base, path = location.pathname + location.search + location.hash) => !path.toLowerCase().indexOf(base.toLowerCase()) ? path.slice(base.length) || '/' : '~' + path + +export const navigatingOnSamePage = (history?: NavigationHistory): boolean => { + const { current, previous } = history || {} + + if (!history) return false + if (!current || !previous) return false + + return current.path.split('#')[0] == previous.path.split('#')[0] +} + +export const shouldScrollToHash = (history?: NavigationHistory): boolean => { + return !(navigatingOnSamePage(history) && history?.current?.replace) +} diff --git a/packages/fastify-renderer/src/client/react/wouter-extension.d.ts b/packages/fastify-renderer/src/client/react/wouter-extension.d.ts new file mode 100644 index 00000000..dae6866f --- /dev/null +++ b/packages/fastify-renderer/src/client/react/wouter-extension.d.ts @@ -0,0 +1,17 @@ +import 'wouter' + +declare module 'wouter' { + export interface RouterProps { + navigationHistory?: NavigationHistory + } + + export interface NavigationHistory { + current?: NavigationHistoryItem + previous?: NavigationHistoryItem + } + + export interface NavigationHistoryItem { + path: string + replace: boolean + } +} From fd2dc86dd8f6f65580249c91554ece243d708d89 Mon Sep 17 00:00:00 2001 From: Daniel Roberts Date: Tue, 9 Aug 2022 13:50:37 -0400 Subject: [PATCH 2/2] fix(navgiation): tests for not scrolling to hash on 'replace' navigation --- .../src/client/react/locationHook.ts | 9 ++ .../simple-react/NavigationHistoryTest.tsx | 98 +++++++++++++++++++ packages/test-apps/simple-react/server.ts | 4 + .../test/navigation-history.spec.ts | 50 ++++++++++ 4 files changed, 161 insertions(+) create mode 100644 packages/test-apps/simple-react/NavigationHistoryTest.tsx create mode 100644 packages/test-apps/simple-react/test/navigation-history.spec.ts diff --git a/packages/fastify-renderer/src/client/react/locationHook.ts b/packages/fastify-renderer/src/client/react/locationHook.ts index 269cf023..ad3d2aef 100644 --- a/packages/fastify-renderer/src/client/react/locationHook.ts +++ b/packages/fastify-renderer/src/client/react/locationHook.ts @@ -21,6 +21,15 @@ export const useTransitionLocation = ({ base = '' } = {}) => { const prevLocation = useRef(path + location.search + location.hash) const [startTransition, isPending] = useTransition() const router = useRouter() + useEffect(() => { + if (!router.navigationHistory) + router.navigationHistory = { + current: { + path, + replace: false, + }, + } + }, []) useEffect(() => { // this function checks if the location has been changed since the diff --git a/packages/test-apps/simple-react/NavigationHistoryTest.tsx b/packages/test-apps/simple-react/NavigationHistoryTest.tsx new file mode 100644 index 00000000..4ed102a3 --- /dev/null +++ b/packages/test-apps/simple-react/NavigationHistoryTest.tsx @@ -0,0 +1,98 @@ +import React from 'react' +import { Link, useLocation } from 'wouter' + +const NavigationHistoryTest = () => { + const [path, navigate] = useLocation() + + return ( + <> +

Navigation Test

+

Leaving this page will set the navigation details on the window for inspection in the tests

+
+ + Go to the content + +
+ + Home + +
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

Another Section

+

+ Lorem ipsum dolor sit amet consectetur adipisicing elit. Fuga earum maiores, excepturi aspernatur perspiciatis + doloribus suscipit voluptates ipsam nam in nostrum vel obcaecati cum illum ex quasi quo est at. +

+ + ) +} + +export default NavigationHistoryTest diff --git a/packages/test-apps/simple-react/server.ts b/packages/test-apps/simple-react/server.ts index 5b8a8c06..489917a2 100644 --- a/packages/test-apps/simple-react/server.ts +++ b/packages/test-apps/simple-react/server.ts @@ -64,6 +64,10 @@ export const server = async () => { return {} }) + server.get('/navigation-history-test', { render: require.resolve('./NavigationHistoryTest') }, async (_request) => { + return {} + }) + await server.register(async (instance) => { instance.setRenderConfig({ document: CustomDocumentTemplate }) diff --git a/packages/test-apps/simple-react/test/navigation-history.spec.ts b/packages/test-apps/simple-react/test/navigation-history.spec.ts new file mode 100644 index 00000000..cc6b98f0 --- /dev/null +++ b/packages/test-apps/simple-react/test/navigation-history.spec.ts @@ -0,0 +1,50 @@ +import { Page } from 'playwright-chromium' +import { newTestPage, reactReady, rootURL } from '../../helpers' + +describe('navigation details', () => { + let page: Page + + beforeEach(async () => { + page = await newTestPage() + await page.goto(`${rootURL}/navigation-history-test`) + await reactReady(page) + }) + + test('navigating to an anchor will scroll down to the anchor', async () => { + const visibleBeforeClick = await isIntersectingViewport(page, '#section') + expect(visibleBeforeClick).toBe(false) + + await page.click('#section-link') + + const visibleAfterClick = await isIntersectingViewport(page, '#section') + expect(visibleAfterClick).toBe(true) + }) + + test('navigating to an anchor that is on the same page via replace: true will not scroll to the anchor', async () => { + const visibleBeforeClick = await isIntersectingViewport(page, '#section') + expect(visibleBeforeClick).toBe(false) + + await page.click('#section-link-replace') + + const visibleAfterClick = await isIntersectingViewport(page, '#section') + expect(visibleAfterClick).toBe(false) + }) +}) + +const isIntersectingViewport = (page: Page, selector: string): Promise => { + return page.$eval(selector, async (element) => { + const visibleRatio: number = await new Promise((resolve) => { + const observer = new IntersectionObserver((entries) => { + resolve(entries[0].intersectionRatio) + observer.disconnect() + }) + observer.observe(element) + // Firefox doesn't call IntersectionObserver callback unless + // there are rafs. + requestAnimationFrame(() => { + /**/ + }) + }) + return visibleRatio > 0 + }) +}