From 72ce65c60501de541583bd20b72d766b957de8cb Mon Sep 17 00:00:00 2001 From: Johann R Date: Wed, 11 Jan 2023 20:31:31 +0100 Subject: [PATCH 1/6] Fix scroll restoration when redirecting in an action (#9815) --- .../scroll-restoration-action-redirect.md | 5 ++ packages/router/__tests__/router-test.ts | 66 +++++++++++++++++++ packages/router/router.ts | 11 ++-- 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 .changeset/scroll-restoration-action-redirect.md diff --git a/.changeset/scroll-restoration-action-redirect.md b/.changeset/scroll-restoration-action-redirect.md new file mode 100644 index 0000000000..186e3c8260 --- /dev/null +++ b/.changeset/scroll-restoration-action-redirect.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Fix scroll restoration when redirecting in an action diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 6e306c7e76..f102ea625b 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -6520,6 +6520,72 @@ describe("a router", () => { expect(t.router.state.preventScrollReset).toBe(false); }); + it("restores scroll on submissions that redirect to the same location", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/tasks"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + 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({}), + }); + 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); + }); + + it("restores scroll on submissions that redirect to new locations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/tasks"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, + }, + }); + + expect(t.router.state.restoreScrollPosition).toBe(false); + expect(t.router.state.preventScrollReset).toBe(false); + let positions = { "/": 50, "/tasks": 100 }; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition, + (l) => l.pathname + ); + + let nav1 = await t.navigate("/tasks", { + formMethod: "post", + formData: createFormData({}), + }); + const nav2 = await nav1.actions.tasks.redirectReturn("/"); + await nav2.loaders.index.resolve("INDEX"); + expect(t.router.state.restoreScrollPosition).toBe(50); + expect(t.router.state.preventScrollReset).toBe(false); + }); + it("does not restore scroll on submissions", async () => { let t = setup({ routes: SCROLL_ROUTES, diff --git a/packages/router/router.ts b/packages/router/router.ts index 74e3cc7d2c..b291dee1a3 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -771,6 +771,12 @@ export function createRouter(init: RouterInit): Router { ) : state.loaderData; + // Don't restore on submission navigations unless they redirect + let restoreScrollPosition = + state.navigation.formData && location.state?._isRedirect !== true + ? false + : getSavedScrollPosition(location, newState.matches || state.matches); + updateState({ ...newState, // matches, errors, fetchers go through as-is actionData, @@ -780,10 +786,7 @@ 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), + restoreScrollPosition, preventScrollReset: pendingPreventScrollReset, }); From 2a385baa3e72fc287597b2e73f4e6103d8dfa4ed Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 15:28:13 -0500 Subject: [PATCH 2/6] Fix up scroll restoration --- .changeset/kind-seals-know.md | 5 + packages/react-router-dom/index.tsx | 2 +- packages/router/__tests__/router-test.ts | 565 +++++++++++++++-------- packages/router/router.ts | 24 +- 4 files changed, 401 insertions(+), 195 deletions(-) create mode 100644 .changeset/kind-seals-know.md 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/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index cf0fd14240..408bcda48f 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -1165,7 +1165,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 f102ea625b..10e6ce7cea 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -6414,232 +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("restores scroll on submissions that redirect to the same location", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/tasks"], - 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 - ); + 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: "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); }); - 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); - }); - it("restores scroll on submissions that redirect to new locations", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/tasks"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", + it("restores scroll on POST submissions", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/tasks"], + hydrationData: { + loaderData: { + index: "INDEX_DATA", + }, }, - }, - }); + }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); - let positions = { "/": 50, "/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); }); - const nav2 = await nav1.actions.tasks.redirectReturn("/"); - await nav2.loaders.index.resolve("INDEX"); - expect(t.router.state.restoreScrollPosition).toBe(50); - 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", - }, - }, - }); - expect(t.router.state.restoreScrollPosition).toBe(false); - expect(t.router.state.preventScrollReset).toBe(false); + describe("scroll reset", () => { + describe("default behavior", () => { + it("resets on navigations", async () => { + let t = setup({ + routes: SCROLL_ROUTES, + initialEntries: ["/"], + 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); - 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(false); - expect(t.router.state.preventScrollReset).toBe(false); - }); + let positions = {}; + let activeScrollPosition = 0; + t.router.enableScrollRestoration( + positions, + () => activeScrollPosition + ); - it("does not reset scroll", async () => { - let t = setup({ - routes: SCROLL_ROUTES, - initialEntries: ["/"], - hydrationData: { - loaderData: { - index: "INDEX_DATA", - }, - }, + 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 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 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); + }); + + 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", { preventScrollReset: true }); - await nav1.loaders.tasks.resolve("TASKS"); - expect(t.router.state.restoreScrollPosition).toBe(null); - expect(t.router.state.preventScrollReset).toBe(true); + 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 b291dee1a3..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,11 +772,13 @@ export function createRouter(init: RouterInit): Router { ) : state.loaderData; - // Don't restore on submission navigations unless they redirect - let restoreScrollPosition = - state.navigation.formData && location.state?._isRedirect !== true - ? false - : getSavedScrollPosition(location, newState.matches || state.matches); + // 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 @@ -786,8 +789,11 @@ export function createRouter(init: RouterInit): Router { initialized: true, navigation: IDLE_NAVIGATION, revalidation: "idle", - restoreScrollPosition, - preventScrollReset: pendingPreventScrollReset, + restoreScrollPosition: getSavedScrollPosition( + location, + newState.matches || state.matches + ), + preventScrollReset, }); if (isUninterruptedRevalidation) { @@ -1775,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 @@ -1788,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, }); } } From f550b69807e207b13a7e01dea6b8124216e52de3 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 15:39:30 -0500 Subject: [PATCH 3/6] Add preventScrollReset prop to Form --- .changeset/quiet-crabs-jump.md | 5 +++++ packages/react-router-dom/dom.ts | 6 ++++++ packages/react-router-dom/index.tsx | 14 ++++++++++++-- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 .changeset/quiet-crabs-jump.md diff --git a/.changeset/quiet-crabs-jump.md b/.changeset/quiet-crabs-jump.md new file mode 100644 index 0000000000..4f63218bbd --- /dev/null +++ b/.changeset/quiet-crabs-jump.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": patch +--- + +Add `preventScrollReset` prop to `
` 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 408bcda48f..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; }; From e12243afe47de43b641b28a29c134d88d37c3ab0 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 15:53:25 -0500 Subject: [PATCH 4/6] Remove dup chageset --- .changeset/scroll-restoration-action-redirect.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/scroll-restoration-action-redirect.md diff --git a/.changeset/scroll-restoration-action-redirect.md b/.changeset/scroll-restoration-action-redirect.md deleted file mode 100644 index 186e3c8260..0000000000 --- a/.changeset/scroll-restoration-action-redirect.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@remix-run/router": patch ---- - -Fix scroll restoration when redirecting in an action From 51c1d5ec9a9976377ef95d8efa005fe00c61d4e7 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 15:53:58 -0500 Subject: [PATCH 5/6] Bundle bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" From 4d7ed68814c57f9b402c5bbb8800b7a5bd29c600 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 15:59:47 -0500 Subject: [PATCH 6/6] Change to minor changeset --- .changeset/quiet-crabs-jump.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/quiet-crabs-jump.md b/.changeset/quiet-crabs-jump.md index 4f63218bbd..9d3b299c36 100644 --- a/.changeset/quiet-crabs-jump.md +++ b/.changeset/quiet-crabs-jump.md @@ -1,5 +1,5 @@ --- -"react-router-dom": patch +"react-router-dom": minor --- Add `preventScrollReset` prop to ``