From ffddf1b410adb718e6e44d21ccd91acfa145ba5b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 30 Aug 2022 15:45:27 -0400 Subject: [PATCH 1/2] fix: rename resetScroll -> preventScrollReset --- docs/components/link.md | 4 +- docs/components/scroll-restoration.md | 6 +- docs/hooks/use-search-params-rn.md | 2 +- docs/hooks/use-search-params.md | 2 +- examples/scroll-restoration/src/routes.tsx | 2 +- .../__tests__/data-browser-router-test.tsx | 63 ++++++++++++++++--- packages/react-router-dom/index.tsx | 20 +++--- packages/react-router-dom/server.tsx | 2 +- packages/react-router/lib/components.tsx | 7 ++- packages/react-router/lib/context.ts | 2 +- packages/router/README.md | 4 +- packages/router/__tests__/router-test.ts | 34 +++++----- packages/router/router.ts | 31 ++++----- 13 files changed, 115 insertions(+), 64 deletions(-) diff --git a/docs/components/link.md b/docs/components/link.md index 680afabb24..8d72ea827a 100644 --- a/docs/components/link.md +++ b/docs/components/link.md @@ -56,12 +56,12 @@ A relative `` value (that does not begin with `/`) resolves relative to [link-native]: ./link-native -## `resetScroll` +## `preventScrollReset` If you are using [``][scrollrestoration], this lets you prevent the scroll position from being reset to the top of the window when the link is clicked. ```tsx - + ``` This does not prevent the scroll position from being restored when the user comes back to the location with the back/forward buttons, it just prevents the reset when the user clicks the link. diff --git a/docs/components/scroll-restoration.md b/docs/components/scroll-restoration.md index 2464508f04..b8dcbcd44f 100644 --- a/docs/components/scroll-restoration.md +++ b/docs/components/scroll-restoration.md @@ -96,10 +96,10 @@ Or you may want to only use the pathname for some paths, and use the normal beha When navigation creates new scroll keys, the scroll position is reset to the top of the page. You can prevent the "scroll to top" behavior from your links: ```tsx - + ``` -See also: [``][resetscroll] +See also: [``][preventscrollreset] ## Scroll Flashing @@ -108,4 +108,4 @@ Without a server side rendering framework like [Remix][remix], you may experienc Server Rendering frameworks can prevent scroll flashing because they can send a fully formed document on the initial load, so scroll can be restored when the page first renders. [remix]: https://remix.run -[resetscroll]: ../components/link#resetscroll +[preventscrollreset]: ../components/link#preventscrollreset diff --git a/docs/hooks/use-search-params-rn.md b/docs/hooks/use-search-params-rn.md index 4465682961..d1b1735c44 100644 --- a/docs/hooks/use-search-params-rn.md +++ b/docs/hooks/use-search-params-rn.md @@ -32,7 +32,7 @@ type SetURLSearchParams = ( interface NavigateOptions { replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; } ``` diff --git a/docs/hooks/use-search-params.md b/docs/hooks/use-search-params.md index ac0ab7055d..23eb46547f 100644 --- a/docs/hooks/use-search-params.md +++ b/docs/hooks/use-search-params.md @@ -32,7 +32,7 @@ type SetURLSearchParams = ( interface NavigateOptions { replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; } ``` diff --git a/examples/scroll-restoration/src/routes.tsx b/examples/scroll-restoration/src/routes.tsx index 2d3e031026..f2125dc411 100644 --- a/examples/scroll-restoration/src/routes.tsx +++ b/examples/scroll-restoration/src/routes.tsx @@ -94,7 +94,7 @@ export function Layout() {
  • - + This link will not scroll to the top
    • diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index e7f9d35e1d..c5f4bb0786 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -537,7 +537,7 @@ function testDomRouter( `); }); - it("handles link navigations with resetScroll=false", async () => { + it("handles link navigations with preventScrollReset", async () => { let { container } = render( }> @@ -551,11 +551,11 @@ function testDomRouter( let state = React.useContext(DataRouterStateContext); return (
    - + Link to Foo Link to Bar -

    {String(state.resetScrollPosition)}

    +

    {String(state?.preventScrollReset)}

    ); @@ -563,25 +563,72 @@ function testDomRouter( fireEvent.click(screen.getByText("Link to Bar")); await waitFor(() => screen.getByText("Bar Heading")); - expect(getHtml(container.querySelector("#resetScrollPosition"))) + expect(getHtml(container.querySelector("#preventScrollReset"))) .toMatchInlineSnapshot(` "

    - true + false

    " `); fireEvent.click(screen.getByText("Link to Foo")); await waitFor(() => screen.getByText("Foo Heading")); - expect(getHtml(container.querySelector("#resetScrollPosition"))) + expect(getHtml(container.querySelector("#preventScrollReset"))) + .toMatchInlineSnapshot(` + "

    + true +

    " + `); + }); + + it("handles link navigations with preventScrollReset={true}", async () => { + let { container } = render( + + }> + Foo Heading} /> + Bar Heading} /> + + + ); + + function Layout() { + let state = React.useContext(DataRouterStateContext); + return ( +
    + + Link to Foo + + Link to Bar +

    {String(state?.preventScrollReset)}

    + +
    + ); + } + + fireEvent.click(screen.getByText("Link to Bar")); + await waitFor(() => screen.getByText("Bar Heading")); + expect(getHtml(container.querySelector("#preventScrollReset"))) .toMatchInlineSnapshot(` "

    false

    " `); + + fireEvent.click(screen.getByText("Link to Foo")); + await waitFor(() => screen.getByText("Foo Heading")); + expect(getHtml(container.querySelector("#preventScrollReset"))) + .toMatchInlineSnapshot(` + "

    + true +

    " + `); }); it("executes route actions/loaders on useSubmit navigations", async () => { diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index d559828824..258f624b3a 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -395,7 +395,7 @@ export interface LinkProps reloadDocument?: boolean; replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; relative?: RelativeRoutingType; to: To; } @@ -413,7 +413,7 @@ export const Link = React.forwardRef( state, target, to, - resetScroll, + preventScrollReset, ...rest }, ref @@ -423,7 +423,7 @@ export const Link = React.forwardRef( replace, state, target, - resetScroll, + preventScrollReset, relative, }); function handleClick( @@ -706,13 +706,13 @@ export function useLinkClickHandler( target, replace: replaceProp, state, - resetScroll, + preventScrollReset, relative, }: { target?: React.HTMLAttributeAnchorTarget; replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; relative?: RelativeRoutingType; } = {} ): (event: React.MouseEvent) => void { @@ -732,7 +732,7 @@ export function useLinkClickHandler( ? replaceProp : createPath(location) === createPath(path); - navigate(to, { replace, state, resetScroll, relative }); + navigate(to, { replace, state, preventScrollReset, relative }); } }, [ @@ -743,7 +743,7 @@ export function useLinkClickHandler( state, target, to, - resetScroll, + preventScrollReset, relative, ] ); @@ -1053,7 +1053,7 @@ function useScrollRestoration({ router != null && state != null, "useScrollRestoration must be used within a DataRouterStateContext" ); - let { restoreScrollPosition, resetScrollPosition } = state; + let { restoreScrollPosition, preventScrollReset } = state; // Trigger manual scroll restoration while we're active React.useEffect(() => { @@ -1125,13 +1125,13 @@ function useScrollRestoration({ } // Opt out of scroll reset if this link requested it - if (resetScrollPosition === false) { + if (preventScrollReset === true) { return; } // otherwise go to the top on new locations window.scrollTo(0, 0); - }, [location, restoreScrollPosition, resetScrollPosition]); + }, [location, restoreScrollPosition, preventScrollReset]); } function useBeforeUnload(callback: () => any): void { diff --git a/packages/react-router-dom/server.tsx b/packages/react-router-dom/server.tsx index df3d5994a0..0fd53d449a 100644 --- a/packages/react-router-dom/server.tsx +++ b/packages/react-router-dom/server.tsx @@ -192,7 +192,7 @@ function getStatelessRemixRouter( initialized: true, navigation: IDLE_NAVIGATION, restoreScrollPosition: null, - resetScrollPosition: true, + preventScrollReset: false, revalidation: "idle" as RevalidationState, fetchers: new Map(), }; diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 1f36ba053e..5adf0a041b 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -88,12 +88,15 @@ export function DataRouterProvider({ createHref: router.createHref, go: (n) => router.navigate(n), push: (to, state, opts) => - router.navigate(to, { state, resetScroll: opts?.resetScroll }), + router.navigate(to, { + state, + preventScrollReset: opts?.preventScrollReset, + }), replace: (to, state, opts) => router.navigate(to, { replace: true, state, - resetScroll: opts?.resetScroll, + preventScrollReset: opts?.preventScrollReset, }), }; }, [router]); diff --git a/packages/react-router/lib/context.ts b/packages/react-router/lib/context.ts index d6d6328f10..f30089d731 100644 --- a/packages/react-router/lib/context.ts +++ b/packages/react-router/lib/context.ts @@ -65,7 +65,7 @@ export type RelativeRoutingType = "route" | "path"; export interface NavigateOptions { replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; relative?: RelativeRoutingType; } diff --git a/packages/router/README.md b/packages/router/README.md index 3419028b31..ef5cd83520 100644 --- a/packages/router/README.md +++ b/packages/router/README.md @@ -45,8 +45,8 @@ interface RouterState { // should not restore,m or null if we don't have a saved position // Note: must be enabled via router.enableScrollRestoration() restoreScrollPosition: number | false | null; - // Proxied `resetScroll` value passed to router.navigate() (default true) - resetScrollPosition: boolean; + // Proxied `preventScrollReset` value passed to router.navigate() (default false) + preventScrollReset: boolean; // Data from the loaders for the current matches loaderData: RouteData; // Data from the action for the current matches diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 16541032c4..42bde9eabf 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -893,7 +893,7 @@ describe("a router", () => { location: undefined, state: "idle", }, - resetScrollPosition: true, + preventScrollReset: false, restoreScrollPosition: null, revalidation: "idle", fetchers: new Map(), @@ -4840,33 +4840,35 @@ describe("a router", () => { }); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); let positions = {}; + + // Simulate scrolling to 100 on / let activeScrollPosition = 100; t.router.enableScrollRestoration(positions, () => activeScrollPosition); - // No restoration on first click to tasks + // No restoration on first click to /tasks let nav1 = await t.navigate("/tasks"); await nav1.loaders.tasks.resolve("TASKS"); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); // Simulate scrolling down on /tasks activeScrollPosition = 200; - // Restore on pop to previous location + // Restore on pop back to / let nav2 = await t.navigate(-1); expect(t.router.state.restoreScrollPosition).toBe(null); await nav2.loaders.index.resolve("INDEX"); expect(t.router.state.restoreScrollPosition).toBe(100); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); - // Forward to /tasks + // Restore on pop forward to /tasks let nav3 = await t.navigate(1); await nav3.loaders.tasks.resolve("TASKS"); expect(t.router.state.restoreScrollPosition).toBe(200); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); }); it("restores scroll using custom key", async () => { @@ -4882,7 +4884,7 @@ describe("a router", () => { }); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); let positions = { "/tasks": 100 }; let activeScrollPosition = 0; @@ -4895,7 +4897,7 @@ describe("a router", () => { let nav1 = await t.navigate("/tasks"); await nav1.loaders.tasks.resolve("TASKS"); expect(t.router.state.restoreScrollPosition).toBe(100); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); }); it("does not restore scroll on submissions", async () => { @@ -4911,7 +4913,7 @@ describe("a router", () => { }); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); let positions = { "/tasks": 100 }; let activeScrollPosition = 0; @@ -4929,7 +4931,7 @@ describe("a router", () => { await nav1.loaders.root.resolve("ROOT"); await nav1.loaders.tasks.resolve("TASKS"); expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); }); it("does not reset scroll", async () => { @@ -4945,18 +4947,16 @@ describe("a router", () => { }); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(true); + expect(t.router.state.preventScrollReset).toBe(false); let positions = {}; let activeScrollPosition = 0; t.router.enableScrollRestoration(positions, () => activeScrollPosition); - let nav1 = await t.navigate("/tasks", { - resetScroll: false, - }); + let nav1 = await t.navigate("/tasks", { preventScrollReset: true }); await nav1.loaders.tasks.resolve("TASKS"); expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.resetScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(true); }); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 0f3501a82e..76a38a7a4f 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -175,10 +175,10 @@ export interface RouterState { restoreScrollPosition: number | false | null; /** - * Indicate whether this navigation should reset the scroll position if we - * are unable to restore the scroll position + * Indicate whether this navigation should skip resetting the scroll position + * if we are unable to restore the scroll position */ - resetScrollPosition: boolean; + preventScrollReset: boolean; /** * Tracks the state of the current navigation @@ -288,7 +288,7 @@ export interface GetScrollPositionFunction { type LinkNavigateOptions = { replace?: boolean; state?: any; - resetScroll?: boolean; + preventScrollReset?: boolean; }; /** @@ -501,7 +501,7 @@ export function createRouter(init: RouterInit): Router { initialized, navigation: IDLE_NAVIGATION, restoreScrollPosition: null, - resetScrollPosition: true, + preventScrollReset: false, revalidation: "idle", loaderData: init.hydrationData?.loaderData || {}, actionData: init.hydrationData?.actionData || null, @@ -512,9 +512,9 @@ export function createRouter(init: RouterInit): Router { // -- Stateful internal variables to manage navigations -- // Current navigation in progress (to be committed in completeNavigation) let pendingAction: HistoryAction = HistoryAction.Pop; - // Should the current navigation reset the scroll position if scroll cannot + // Should the current navigation prevent the scroll reset if scroll cannot // be restored? - let pendingResetScroll = true; + let pendingPreventScrollReset = false; // AbortController for the active navigation let pendingNavigationController: AbortController | null; // We use this to avoid touching history in completeNavigation if a @@ -649,8 +649,7 @@ export function createRouter(init: RouterInit): Router { restoreScrollPosition: state.navigation.formData ? false : getSavedScrollPosition(location, newState.matches || state.matches), - // Always reset scroll unless explicitly told not to - resetScrollPosition: pendingResetScroll, + preventScrollReset: pendingPreventScrollReset, }); if (isUninterruptedRevalidation) { @@ -665,7 +664,7 @@ export function createRouter(init: RouterInit): Router { // Reset stateful navigation vars pendingAction = HistoryAction.Pop; - pendingResetScroll = true; + pendingPreventScrollReset = false; isUninterruptedRevalidation = false; isRevalidationRequired = false; cancelledDeferredRoutes = []; @@ -690,15 +689,17 @@ export function createRouter(init: RouterInit): Router { opts?.replace === true || submission != null ? HistoryAction.Replace : HistoryAction.Push; - let resetScroll = - opts && "resetScroll" in opts ? opts.resetScroll : undefined; + let preventScrollReset = + opts && "preventScrollReset" in opts + ? opts.preventScrollReset === true + : undefined; return await startNavigation(historyAction, location, { submission, // Send through the formData serialization error if we have one so we can // render at the right error boundary after we match routes pendingError: error, - resetScroll, + preventScrollReset, replace: opts?.replace, }); } @@ -747,7 +748,7 @@ export function createRouter(init: RouterInit): Router { overrideNavigation?: Navigation; pendingError?: ErrorResponse; startUninterruptedRevalidation?: boolean; - resetScroll?: boolean; + preventScrollReset?: boolean; replace?: boolean; } ): Promise { @@ -762,7 +763,7 @@ export function createRouter(init: RouterInit): Router { // Save the current scroll position every time we start a new navigation, // and track whether we should reset scroll on completion saveScrollPosition(state.location, state.matches); - pendingResetScroll = opts?.resetScroll !== false; + pendingPreventScrollReset = opts?.preventScrollReset === true; let loadingNavigation = opts?.overrideNavigation; let matches = matchRoutes(dataRoutes, location, init.basename); From b3da6dfc0b926103ae2382d58c63834c4ceda6b9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 30 Aug 2022 15:46:28 -0400 Subject: [PATCH 2/2] Add changeset --- .changeset/red-sheep-push.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/red-sheep-push.md diff --git a/.changeset/red-sheep-push.md b/.changeset/red-sheep-push.md new file mode 100644 index 0000000000..11cab920fc --- /dev/null +++ b/.changeset/red-sheep-push.md @@ -0,0 +1,7 @@ +--- +"react-router": patch +"react-router-dom": patch +"@remix-run/router": patch +--- + +fix: rename resetScroll -> preventScrollReset (#9199)