-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[RFR] Fix create to edit form values #2339
Conversation
import { LOCATION_CHANGE } from 'react-router-redux'; | ||
import { beforeLocationChange } from '../actions/formActions'; | ||
|
||
const formMiddleware = () => next => action => { |
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.
I and other developers will be unable to understand why you're doing that. This file requires many, many comments to describe the issue and the solution.
a1a93bc
to
cc16e80
Compare
a8abdc7
to
db7b3c3
Compare
db7b3c3
to
d9b45d2
Compare
Oh no, now that it is RFR and green, that's the moment you say "Can you target next?", right @fzaninotto? 😭 |
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.
I think you must update the CustomApp doc, too.
I'm OK to commit it to master, as the bug is important.
* | ||
* A middleware is needed instead of a saga because we need to control the actions | ||
* order: we need to ensure we reset the redux form BEFORE the location actually | ||
* change. Otherwise, the new page which may contains a record redux-form might |
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.
s/change/changes/
s/may contains/may contain/
As requested by fzaninotto at #2339 (comment)
As requested by fzaninotto at #2339 (comment)
Actually the |
Awesome, this makes it a non-BC break change, and therefore totally worthy of the |
Fixes #2324