Allow empty option for enum fields #451
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
n1k0
approved these changes
Jan 25, 2017
| @@ -181,6 +206,19 @@ describe("StringField", () => { | |||
| expect(comp.state.formData).eql("foo"); | |||
| }); | |||
|
|
|||
| it("should reflect undefined into form state is empty option selected", () => { | |||
| @@ -7,7 +7,9 @@ import {asNumber} from "../../utils"; | |||
| * always retrieved as strings. | |||
| */ | |||
| function processValue({type, items}, value) { | |||
| if (type === "array" && items && ["number", "integer"].includes(items.type)) { | |||
| if (value === "") { | |||
| return undefined; | |||
There was a problem hiding this comment.
We need to add a warning somewhere in the docs, because if one adds "" as an enum choice in their schema, it will be automatically converted to undefined and drop the property from any parent object - which is now consistent with our text inputs but may be surprising to users.
There was a problem hiding this comment.
I added a note, but I'm not sure how well I explained it or if there's a better spot.
n1k0
reviewed
Jan 27, 2017
| @@ -681,6 +682,10 @@ Form component supports the following html attributes: | |||
| schema={} /> | |||
| ``` | |||
|
|
|||
| ### Enum fields | |||
|
|
|||
| String fields that use `enum` and a `select` widget will have an empty option in the options list. When a user selects that option, the field will be set to `undefined` (similar to how regular `string` fields work if the field is empty). This also means that if you have an empty string in your `enum` array, selecting that option will cause the field to be set to `undefined`. | |||
There was a problem hiding this comment.
I think I'd move this in new section under Form data validation, eg. The case of empty strings.
|
Perfect, thank you! |
n1k0
added a commit
that referenced
this pull request
Jan 28, 2017
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reasons for making this change
This removes the logic that defaults enum fields to the first item and updates the date and select widgets to use an empty default option when a field is undefined.
This ended up being more difficult to get right at the widget level than I thought and I've essentially skipped making this change affect
multiplearrays. This needs some careful checking for the cases that aren't on the simpler string field using a select widget path.Closes #449.
Checklist