-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
Use updated value for formatOnBlur
format func
#465
Use updated value for formatOnBlur
format func
#465
Conversation
Things have changed with v5, but if this is still a problem, we can address it. I'm not understanding your concern. In your demo, you should just be doing the |
I've updated to v5 and this still seems to be an issue. |
With the example, it wouldn't allow spaces to be typed. |
37af286
to
ac0b686
Compare
Codecov Report
@@ Coverage Diff @@
## master #465 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 14 16 +2
Lines 231 225 -6
Branches 57 55 -2
=====================================
- Hits 231 225 -6
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #465 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 10 -5
Lines 235 210 -25
Branches 58 62 +4
=====================================
- Hits 235 210 -25
Continue to review full report at Codecov.
|
ac0b686
to
4850cb2
Compare
@erikras Thanks for checking into this! |
I don't doubt that this fixes your problem, but I'd like to have a test that demonstrates the problem and fails with this code commented out, but passes with it included. Either you can do it, or I can try to get around to it later this week... |
Your test is pretty good, but it's not, the parlance of the React Testing Library motto, "how a user would use the library". Primarily: |
Why, why, why you'd want to call imperatively onChange inside the onBlur? this sounds like a misusage to me, but maybe I'm just not aware about the use case - would be good to describe it 🤔 |
I agree that this feels wrong somehow. I'll have to give more thought to the motives behind this change. It also created an untested code branch. I'm not sure when the ternary on line 115 would ever flip the other way. |
Published in |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
When there's a
format
function that's calledonBlur
it passes thecurrent value from state to that
format
function.react-final-form/src/Field.js
Line 113 in 41d24d6
This causes that when the field's
onChange
is called from within theonBlur
the
formatOnBlur
function is not getting the updated value.Rather, it has the previous value passed to it, so when
formatting the value it ends up formatting and returning the previous value.
This pr changes to pass the current updated value from
props.reactFinalForm
's state,to the format func so it always gets the current value.
See Demo