-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 #1424: Set value for boolean field without default value #2525
Conversation
@chrfritsch could you add a simple test to confirm this behavior in the |
Maybe something like this? It's my first jest test I ever wrote :D I don't understand why it returns an object with title: false. Maybe there is something else broken? But without the fix the object was empty. |
{ | ||
name: 'post', | ||
widget: 'boolean', | ||
fields: [{ name: 'title', widget: 'boolean' }], |
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.
boolean
widgets don't have fields
attrs, test for { name: 'boolean', widget: 'boolean' }
for example, which should output { boolean: false }
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 switched the parent back to object, like all the other tests. That was a leftover from some back and forth testing...
// A boolean field without a default value defined will initially render | ||
// render as false, but the field will fail the required check. So we are | ||
// assinging false. | ||
const widgetDefaultValue = item.get('widget') == 'boolean' ? false : null; |
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.
We really want to avoid hardcoding widget names. The underlying issue here is bigger than just the boolean widget.
Thanks for this @chrfritsch! I'm not sure it's the right approach, though. What we really need is for widgets to set their own default values. There's some discussion around this in #1407, but it isn't fully fleshed out yet. Going to close this, but please comment if you have any thoughts on it or questions! |
Summary
This fixes a validation issue for boolean fields without any default value for new posts.
Test plan
A picture of a cute animal (not mandatory but encouraged)