-
Notifications
You must be signed in to change notification settings - Fork 123
Bugfix: updateInputsWithValue doesn't update nested field #596
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
Conversation
adsee42
left a comment
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.
Looks great👍
Didn't know about lodash's get and set.😋
btw, (ignore this if I was wrong) this function looks and works very similarly to updateInputsWithError, and how they are implemented seems different. wondering why is the difference
|
@adsee42 You're right. I thought a lot about this and I'm on the fence. Should you be able to pass object-style error messages like The inputs with value one makes more sense, since we output an object at the end. Of course none of this would matter except that |
|
@rkuykendall |
|
Yeah it feels like an odd feature. I guess the idea is that if you're passing errors to the form and they're not in the form, that's bad and we should let you know. |
|
I'm going to merge the change as-is and leave the questions about input errors for another conversation. |
|
@ThibautMarechal or @felixmosh Can I get a code review if you're free? |
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.
Looks OK to me :]
( what is the feature, or what is it this is PR fixing? )
CHANGELOG.mdwhen we do a releaseAPI.mddist/changes have not been committed.Tests run directly on the source code and all dist changes are computed and committed during Formsy release.