-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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 warn when unsaved changes #6719
Conversation
Object.keys(unsavedChanges).forEach(key => | ||
form.change(key, unsavedChanges[key]) | ||
const release = history.block(location => { | ||
const isInsideForm = location.pathname.startsWith( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true e.g. on a TabbedForm when the user arrives directly in tab 2 and navigates to tab 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, you're right. I'll have to figure out how to determine the form root path then
The |
const form = useForm(); | ||
const useWarnWhenUnsavedChanges = ( | ||
enable: boolean, | ||
formRootPathname: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a breaking change: you add a second compulsory parameter, which isn't set in e.g. SimpleForm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because FormWithRedirect set a default value when it's not provided
Can you elaborate? |
react-router v6 removed useHistory history.block is not work |
Closes #6678.
Tasks
Description
The linked issue above was introduced by #6272 that fixed the issue ##6198 introduced by #6005.
However, all those workarounds were in fact necessary because of our implementation for warning users about potential data loss if they tried to leave a page with a form with modified fields. Indeed, the
useWarnWhenUnsavedChanges
was saving the form changes when the form was unmounted because a location change happened. If users cancelled the navigation, then the hook would renavigate to the previous location and reapplied the saved changes.However, re-navigating to the previous location means we refetched the data, reapplied initial values for the loaded record (in case of edit) then reapplied the saved changes. All this lead to several workarounds to manage dirty states that introduced more problems.
This PR uses the history API to trigger the warning. This is better because it happens before the location change so we don't have to save changes and reapply them. This allows us to remove the workarounds.