diff --git a/.changeset/kind-seals-know.md b/.changeset/kind-seals-know.md new file mode 100644 index 0000000000..9f6d2fde4f --- /dev/null +++ b/.changeset/kind-seals-know.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Fix scroll reset if a submission redirects diff --git a/.changeset/quiet-crabs-jump.md b/.changeset/quiet-crabs-jump.md new file mode 100644 index 0000000000..9d3b299c36 --- /dev/null +++ b/.changeset/quiet-crabs-jump.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": minor +--- + +Add `preventScrollReset` prop to `
` diff --git a/package.json b/package.json index 037d859131..e69c3e3a5e 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "38.5 kB" + "none": "39 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "12.5 kB" diff --git a/packages/react-router-dom/dom.ts b/packages/react-router-dom/dom.ts index e6fd5aa73f..21d9b84a15 100644 --- a/packages/react-router-dom/dom.ts +++ b/packages/react-router-dom/dom.ts @@ -138,6 +138,12 @@ export interface SubmitOptions { * hierarchy and want to instead route based on /-delimited URL segments */ relative?: RelativeRoutingType; + + /** + * In browser-based environments, prevent resetting scroll after this + * navigation when using the component + */ + preventScrollReset?: boolean; } export function getFormSubmissionInfo( diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index cf0fd14240..104994775e 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -593,6 +593,12 @@ export interface FormProps extends React.FormHTMLAttributes { */ relative?: RelativeRoutingType; + /** + * Prevent the scroll position from resetting to the top of the viewport on + * completion of the navigation when using the component + */ + preventScrollReset?: boolean; + /** * A function to call when the form is submitted. If you call * `event.preventDefault()` then this form will not do anything. @@ -640,6 +646,7 @@ const FormImpl = React.forwardRef( fetcherKey, routeId, relative, + preventScrollReset, ...props }, forwardedRef @@ -664,6 +671,7 @@ const FormImpl = React.forwardRef( method: submitMethod, replace, relative, + preventScrollReset, }); }; @@ -906,6 +914,7 @@ function useSubmitImpl(fetcherKey?: string, routeId?: string): SubmitFunction { let href = url.pathname + url.search; let opts = { replace: options.replace, + preventScrollReset: options.preventScrollReset, formData, formMethod: method as FormMethod, formEncType: encType as FormEncType, @@ -1000,8 +1009,9 @@ export type FetcherWithComponents = Fetcher & { Form: ReturnType; submit: ( target: SubmitTarget, - // Fetchers cannot replace because they are not navigation events - options?: Omit + // Fetchers cannot replace/preventScrollReset because they are not + // navigation events + options?: Omit ) => void; load: (href: string) => void; }; @@ -1165,7 +1175,7 @@ function useScrollRestoration({ } } - // Opt out of scroll reset if this link requested it + // Don't reset if this navigation opted out if (preventScrollReset === true) { return; } diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 6e306c7e76..10e6ce7cea 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -6414,166 +6414,423 @@ describe("a router", () => { }, ]; - it("restores scroll on initial load (w/o hydrationData)", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/no-loader"], - }); + describe("scroll restoration", () => { + it("restores scroll on initial load (w/o hydrationData)", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/no-loader"], + }); - expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.preventScrollReset).toBe(false); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(false); - // Assume initial location had a saved position - let positions = { default: 50 }; - t.router.enableScrollRestoration(positions, () => 0); - expect(t.router.state.restoreScrollPosition).toBe(50); - }); + // Assume initial location had a saved position + let positions = { default: 50 }; + t.router.enableScrollRestoration(positions, () => 0); + expect(t.router.state.restoreScrollPosition).toBe(50); + }); - it("restores scroll on initial load (w/ hydrationData)", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX", + it("restores scroll on initial load (w/ hydrationData)", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX", + }, }, - }, - }); + }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); - // Assume initial location had a saved position - let positions = { default: 50 }; - t.router.enableScrollRestoration(positions, () => 0); - expect(t.router.state.restoreScrollPosition).toBe(false); - }); + // Assume initial location had a saved position + let positions = { default: 50 }; + t.router.enableScrollRestoration(positions, () => 0); + expect(t.router.state.restoreScrollPosition).toBe(false); + }); - it("restores scroll on navigations", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", + it("restores scroll on navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, }, - }, - }); + }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); - let positions = {}; + let positions = {}; - // Simulate scrolling to 100 on / - let activeScrollPosition = 100; - t.router.enableScrollRestoration(positions, () => activeScrollPosition); + // Simulate scrolling to 100 on / + let activeScrollPosition = 100; + t.router.enableScrollRestoration(positions, () => activeScrollPosition); - // 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.preventScrollReset).toBe(false); + // 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.preventScrollReset).toBe(false); - // Simulate scrolling down on /tasks - activeScrollPosition = 200; + // Simulate scrolling down on /tasks + activeScrollPosition = 200; - // 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.preventScrollReset).toBe(false); + // 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.preventScrollReset).toBe(false); - // 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.preventScrollReset).toBe(false); - }); + // 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.preventScrollReset).toBe(false); + }); - it("restores scroll using custom key", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", + it("restores scroll using custom key", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, }, - }, - }); + }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); - let positions = { "/tasks": 100 }; - let activeScrollPosition = 0; - t.router.enableScrollRestoration( - positions, - () => activeScrollPosition, - (l) => l.pathname - ); + let positions = { "/tasks": 100 }; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition, + (l) => l.pathname + ); - let nav1 = await t.navigate("/tasks"); - await nav1.loaders.tasks.resolve("TASKS"); - expect(t.router.state.restoreScrollPosition).toBe(100); - expect(t.router.state.preventScrollReset).toBe(false); - }); + let nav1 = await t.navigate("/tasks"); + await nav1.loaders.tasks.resolve("TASKS"); + expect(t.router.state.restoreScrollPosition).toBe(100); + expect(t.router.state.preventScrollReset).toBe(false); + }); - it("does not restore scroll on submissions", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", + it("restores scroll on GET submissions", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/tasks"], + hydrationData: { + loaderData: { + tasks: "TASKS", + }, }, - }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + // We were previously on tasks at 100 + let positions = { "/tasks": 100 }; + // But we've scrolled up to 50 to submit. We'll save this overtop of + // the 100 when we start this submission navigation and then restore to + // 50 below + let activeScrollPosition = 50; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition, + (l) => l.pathname + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "get", + formData: createFormData({ key: "value" }), + }); + await nav1.loaders.tasks.resolve("TASKS2"); + expect(t.router.state.restoreScrollPosition).toBe(50); + expect(t.router.state.preventScrollReset).toBe(false); }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + it("restores scroll on POST submissions", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/tasks"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); - let positions = { "/tasks": 100 }; - let activeScrollPosition = 0; - t.router.enableScrollRestoration( - positions, - () => activeScrollPosition, - (l) => l.pathname - ); + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + // We were previously on tasks at 100 + let positions = { "/tasks": 100 }; + // But we've scrolled up to 50 to submit. We'll save this overtop of + // the 100 when we start this submission navigation and then restore to + // 50 below + let activeScrollPosition = 50; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition, + (l) => l.pathname + ); - let nav1 = await t.navigate("/tasks", { - formMethod: "post", - formData: createFormData({}), + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + }); + const nav2 = await nav1.actions.tasks.redirectReturn("/tasks"); + await nav2.loaders.tasks.resolve("TASKS"); + expect(t.router.state.restoreScrollPosition).toBe(50); + expect(t.router.state.preventScrollReset).toBe(false); }); - await nav1.actions.tasks.resolve("ACTION"); - await nav1.loaders.tasks.resolve("TASKS"); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); }); - it("does not reset scroll", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", - }, - }, + describe("scroll reset", () => { + describe("default behavior", () => { + it("resets on navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks"); + await nav1.loaders.tasks.resolve("TASKS"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(false); + }); + + it("resets on navigations that redirect", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks"); + let nav2 = await nav1.loaders.tasks.redirectReturn("/"); + await nav2.loaders.index.resolve("INDEX_DATA 2"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(false); + }); + + it("does not reset on submission navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + }); + await nav1.actions.tasks.resolve("ACTION"); + await nav1.loaders.tasks.resolve("TASKS"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(true); + }); + + it("resets on submission navigations that redirect", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + }); + let nav2 = await nav1.actions.tasks.redirectReturn("/"); + await nav2.loaders.index.resolve("INDEX_DATA2"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(false); + }); }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + describe("user-specified flag preventScrollReset flag", () => { + it("prevents scroll reset on navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); - let positions = {}; - let activeScrollPosition = 0; - t.router.enableScrollRestoration(positions, () => activeScrollPosition); + 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.preventScrollReset).toBe(true); + }); + + it("prevents scroll reset on navigations that redirect", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks", { preventScrollReset: true }); + let nav2 = await nav1.loaders.tasks.redirectReturn("/"); + await nav2.loaders.index.resolve("INDEX_DATA 2"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(true); + }); - 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.preventScrollReset).toBe(true); + it("prevents scroll reset on submission navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + preventScrollReset: true, + }); + await nav1.actions.tasks.resolve("ACTION"); + await nav1.loaders.tasks.resolve("TASKS"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(true); + }); + + it("prevents scroll reset on submission navigations that redirect", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + preventScrollReset: true, + }); + let nav2 = await nav1.actions.tasks.redirectReturn("/"); + await nav2.loaders.index.resolve("INDEX_DATA2"); + expect(t.router.state.restoreScrollPosition).toBe(null); + expect(t.router.state.preventScrollReset).toBe(true); + }); + }); }); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 74e3cc7d2c..018078f79d 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -371,6 +371,7 @@ type LinkNavigateOptions = { type SubmissionNavigateOptions = { replace?: boolean; state?: any; + preventScrollReset?: boolean; formMethod?: FormMethod; formEncType?: FormEncType; formData: FormData; @@ -771,6 +772,14 @@ export function createRouter(init: RouterInit): Router { ) : state.loaderData; + // Always respect the user flag. Otherwise don't reset on mutation + // submission navigations unless they redirect + let preventScrollReset = + pendingPreventScrollReset === true || + (state.navigation.formMethod != null && + isMutationMethod(state.navigation.formMethod) && + location.state?._isRedirect !== true); + updateState({ ...newState, // matches, errors, fetchers go through as-is actionData, @@ -780,11 +789,11 @@ export function createRouter(init: RouterInit): Router { initialized: true, navigation: IDLE_NAVIGATION, revalidation: "idle", - // Don't restore on submission navigations - restoreScrollPosition: state.navigation.formData - ? false - : getSavedScrollPosition(location, newState.matches || state.matches), - preventScrollReset: pendingPreventScrollReset, + restoreScrollPosition: getSavedScrollPosition( + location, + newState.matches || state.matches + ), + preventScrollReset, }); if (isUninterruptedRevalidation) { @@ -1772,6 +1781,8 @@ export function createRouter(init: RouterInit): Router { ...submission, formAction: redirect.location, }, + // Preserve this flag across redirects + preventScrollReset: pendingPreventScrollReset, }); } else { // Otherwise, we kick off a new loading navigation, preserving the @@ -1785,6 +1796,8 @@ export function createRouter(init: RouterInit): Router { formEncType: submission ? submission.formEncType : undefined, formData: submission ? submission.formData : undefined, }, + // Preserve this flag across redirects + preventScrollReset: pendingPreventScrollReset, }); } }