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

Adds action info to transition #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jwickens
Copy link
Collaborator

@jwickens jwickens commented Jun 9, 2021

willTransferFrom in a route handler can be used to catch transitions and abort them if the user does not actually want to leave the page. When this transition is caused by using the forward and backwards buttons in the browser, react router has to undo this. Currently it does it by using History.back() which is problematic when the browser has already gone back. Instead it needs to go forward or redirect back to the original page.

This issue is documented in the original repo in issues like this one: remix-run/react-router#1613. The react training team later solved by releasing version 1.0.0, which besides other major changes also uses the history dependency.

This PR is for https://github.com/meadow/admin/pull/731/commits/57cdb6179b4917c4394eda9be95608b68000cd09

PR meadow/admin#731 does not need to be updated on merge as meadow/admin#731 is pointing to a version tag.

The solution implemented here adds an extra meta property containing previous route location (ie current after the user clicks back) to the transition object. This allows the listener for route changes in the object to use the meta to redirect back to the original page, instead of relying on abort which calls History.back. Abort is still used when the action causing the transition is not a pop. Going backwards or forwards is a pop action whereas click a link is a push action.

@jwickens
Copy link
Collaborator Author

Oops before we close this branch we need to update admin to point at a version tag rather than the branch.

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

Successfully merging this pull request may close these issues.

1 participant