Skip to content
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

Enable the required field on arrays and radio buttons #411

Closed
2 tasks done
crumblix opened this issue Nov 23, 2016 · 5 comments
Closed
2 tasks done

Enable the required field on arrays and radio buttons #411

crumblix opened this issue Nov 23, 2016 · 5 comments
Labels

Comments

@crumblix
Copy link
Collaborator

crumblix commented Nov 23, 2016

Prerequisites

  • I have read the documentation;
  • In the case of a bug report, I understand that providing a SSCCE example is tremendously useful to the maintainers.

Description

Following on from my last suggestion:

#410

I'd like to suggest enabling the "required" field on arrays and radio buttons.

If we make the changes I suggested in the above issue, we can enabled the "required" field on arrays. The use case would be where we want the user to select "at least one" checkbox in a list.

Also, the RadioWidget input supports the "required" tag, which in addition to being a nicer prompt for the user is also consistent with it's usage elsewhere in the module.

Steps to Reproduce

  1. [First Step]
    Attempt to make a radio button list mandatory, or an array mandatory.
  2. [Second Step]
    For the radio widget instead of a clean "please fill in the field" message that we get when we use the mandatory tag, we get the errorlist returned.
    If issue 410 is also addressed then we can make an array mandatory. Without 410 an empty array is still an array and so it won't work. With it we can make an array mandatory. We can't use the "required" HTML attribute, but at least the validation routines will return and prompt us with the error of the missing selection.

Expected behavior

For radio buttons: They should act like any other input that supports the "required" attribute.
For arrays (with 410): We should be able to make it possible to force the user to select "at least one" element in an array.

Actual behavior

Radio buttons go straight to the validation routine. While it works it's not as neat or consistent IMO.
You can't have mandatory arrays currently at all.

Version

Latest

Again here is the change in my fork:
Commit: a020837 [a020837]

@n1k0
Copy link
Collaborator

n1k0 commented Feb 13, 2017

We've discussed this in the past already, I believe the jsonschema spec (which drives this lib implementation) wants minItems to be used to define a required array to have at least n items attached to an array field. I'm not sure this lib should override the spec with its own opinions for achieving the same results using different ways to express their constraints.

But maybe I'm missing subtleties here; could you please create a jsfiddle highlighting the specific use case we're trying to cover here? Thanks!

@n1k0 n1k0 added the question label Feb 13, 2017
@crumblix
Copy link
Collaborator Author

crumblix commented Feb 13, 2017

WIth regards arrays ... agreed. When this was raised I wan't aware of minItems. This has been resolved with better documentation.

The other part of this particular Issue was with regards Radio Options. HTML allows for this control to use the "required" tag, and I think for consistency it should. The validation routine still works of course, but we're missing the additional HTML validation layer that other similar controls have.

The specific change (if it was agreed) would be a one-liner ... in RadioWidget.js:

<input type="radio"
              checked={checked}
              name={name}
              required={required}    <-------
...

@n1k0
Copy link
Collaborator

n1k0 commented Feb 13, 2017

Oh, I see. Indeed, having native HTML5 validation for required radio boolean fields would be nice. We'd just need to ensure handling the recently introduced noHtml5Validate form prop.

Could you create a PR from your commit? Thanks!

@crumblix
Copy link
Collaborator Author

No problem will do :)

crumblix added a commit to crumblix/react-jsonschema-form that referenced this issue Feb 14, 2017
@n1k0 n1k0 closed this as completed in 8b2f966 Feb 14, 2017
n1k0 added a commit that referenced this issue Feb 22, 2017
Highlights
---

- Improved performance and reactivity.
- More consistent validation behavior and UX for array field items.

Backward incompatible changes
---

- `ObjectField` and `ArrayField` are now stateless components, their
  local state handling has been dropped entirely, resulting in large
  performance improvements.
- The `defaultFieldValue` helper from the `utils` module has been
  removed, as it wasn't used anymore.

New features
---

* Fix #411:  Enable required field on radio options. (#469)
* Spread `formContext` to `ArrayTemplateField`, `files` and `multiselect`
  arrays (#456)
* From #471: Non-nullable array item fields are now marked as required in
  the UI.

Bugfixes
---

* Don't pass consumed `SchemaField` class names to child component (#439)
* Turn `ObjectField` and `ArrayField` into stateless components (#480)
* Fix #471: Drop default value initialization at the widget level. (#476)

Kudos
---

Special thanks to @crumblix and @knilink for their help on this release.
You guys rock!
@n1k0
Copy link
Collaborator

n1k0 commented Feb 22, 2017

Released in v0.43.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants