Add onBlur event #431
Add onBlur event #431
Conversation
|
I like the approach of this patch better than the proposed implementation in #444. I think it's worth having the field unique id passed as the handler first argument as it's the most obvious way to know which field has triggered the event. Could you please rebase your patch against latest master so we can land this? |
| @@ -439,6 +447,7 @@ if (process.env.NODE_ENV !== "production") { | |||
| idSchema: PropTypes.object, | |||
| errorSchema: PropTypes.object, | |||
| onChange: PropTypes.func.isRequired, | |||
| onBlur: PropTypes.func.isRequired, | |||
There was a problem hiding this comment.
We should probably leave this optional and test for its presence instead.
There was a problem hiding this comment.
I think I can just remove the isRequired part here. There's no actual use of onBlur in this component, it's just passed through.
|
@n1k0 I'm assuming that there's always an onBlur function passed down to the widget components, and I have the conditional check for a user specified onBlur function in the Form component. I could add a conditional in front of the use of onBlur in the widget components instead (I think the other PR did this). Do you have a preference? |
Actually I don't have a strong opinion on this. While the |
| @@ -24,7 +25,8 @@ function BaseInput(props) { | |||
| readOnly={readonly} | |||
| autoFocus={autofocus} | |||
| value={typeof value === "undefined" ? "" : value} | |||
| onChange={_onChange} /> | |||
| onChange={_onChange} | |||
| onBlur={onBlur ? (event) => onBlur(inputProps.id, event.target.value) : undefined}/> | |||
There was a problem hiding this comment.
I realize we could just do onBlur && (event) => onBlur(inputProps.id, event.target.value) here, but that may be harder to read. Opinions?
There was a problem hiding this comment.
I think that's better, actually.
| } | ||
|
|
||
| return newValue; | ||
| } |
There was a problem hiding this comment.
Let's take the opportunity to improve readability here:
function getValue(event, multiple) {
if (multiple) {
return [].slice.call(event.target.options)
.filter(o => o.selected)
.map(o => o.value);
} else {
return event.target.value;
}
}
|
Great work, thank you. |
Breaking changes When a text input is emptied by the user, it's value is now reset to `undefined` instead of being set to `""` (empty string) as previously. This better matches traditional HTML forms behavior. New features * Add an array field template component (#437) * Wrap radio and checkbox labels into span to enable styling. (#428) * Reset text inputs value when emptied (#442) * Add transform hook for changing errors from json schema validation (#432) * Add a noHtml5Validate prop (#448) * Add support for a onBlur event handler (#431) * Allow empty option for enum fields (#451) Bugfixes * Fix #452: Support recursively referenced definitions. (#453) * Fix #454: Document what master actually is, suffix its version with -dev.
|
Released in v0.42.0. |
Reasons for making this change
We want to be able to show validation errors only after a user has touched a field or tried to submit. We can do the submit part easily through the onError hook and formContext, but we need a way to track blur events. This adds that, similar to the onChange hook.
Should close #364
Checklist