Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix scroll restoration when redirecting in an action #9886

Merged
merged 6 commits into from Jan 11, 2023

Conversation

brophdawg11
Copy link
Contributor

Closes #9577

This also introduces <Form preventScrollReset> and fixes an underlying bug where we were preventing restore on submissions when we should be preventing reset 😬 . We always restore if we have a previous known position (which we never do by default because we use location.key).

New semantics:

// 1. Default behavior

// Resets scroll
<Link />
<Form />
<Form method="get" />
<Form method="post" /> /* action redirects */

// Does not reset scroll
<Form method="post" /> /* action does not redirect */


// 2. Override default

// does not reset scroll
<Link preventScrollReset />
<Form preventScrollReset />
<Form preventScrollReset method="get" />
<Form preventScrollReset method="post" /> /* action redirects */
<Form preventScrollReset method="post" /> /* action does not redirect */

@changeset-bot
Copy link

changeset-bot bot commented Jan 11, 2023

🦋 Changeset detected

Latest commit: 4d7ed68

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router-dom Minor
react-router Minor
react-router-dom-v5-compat Minor
react-router-native Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines +792 to +795
restoreScrollPosition: getSavedScrollPosition(
location,
newState.matches || state.matches
),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always try to restore - that never should have been conditional on submissions

Comment on lines +779 to +781
(state.navigation.formMethod != null &&
isMutationMethod(state.navigation.formMethod) &&
location.state?._isRedirect !== true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, don't reset on non-redirecting submission navigations

@brophdawg11 brophdawg11 merged commit 0582210 into release-next Jan 11, 2023
@brophdawg11 brophdawg11 deleted the brophdawg11/action-scroll-restoration branch January 11, 2023 21:02
@nrako
Copy link

nrako commented Jan 21, 2023

@brophdawg11 – this is great but unfortunately preventScrollReset doesn't work on redirect when the FormImpl is fetcher.Form, I've opened an issue #9961 and put up this stackblitz to reproduce the issue https://stackblitz.com/edit/node-sbijvk?file=app/routes/index.tsx

@brophdawg11
Copy link
Contributor Author

Thanks @nrako, I transferred that issue to the react router repo and will try to take a look this week 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants