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

Nested non-required object validation does not work #2103

Open
selaux opened this issue Feb 17, 2019 · 15 comments
Open

Nested non-required object validation does not work #2103

selaux opened this issue Feb 17, 2019 · 15 comments
Labels

Comments

@selaux
Copy link
Contributor

selaux commented Feb 17, 2019

Describe the bug

When an non-required object inside an object has required fields, the object widgets validation ignores the non-required attribute of the object and highlights the required sub fields of the non-required object as required.

To Reproduce

  1. Use configuration below
  2. Create New Something
  3. Only fill title string

Expected behavior

Entry saved correctly because some-metadata-property is not required.

Current behavior

Entry is not saved and property-specific-string is shown as required.

Screenshots

output

Applicable Versions:

  • Netlify CMS version: 2.4.2

CMS configuration

backend:
  name: test-repo
media_folder: assets
collections: # A list of collections the CMS should be able to edit
  - name: something
    label: 'Something'
    folder: nested
    create: true
    fields:
      - name: title
        widget: string
      - name: metadata
        widget: object
        fields:
          - name: some-metadata-property
            widget: object
            required: false
            fields:
              - name: property-specific-string
                widget: string
@selaux selaux changed the title Nested optional object validation does not work Nested non-required object validation does not work Feb 17, 2019
@barthc
Copy link
Contributor

barthc commented Feb 17, 2019

@selaux could you please edit your config file like so:

fields:
  - name: some-metadata-property
    widget: object
    fields:
      - name: property-specific-string
        widget: string
        required: false

@selaux
Copy link
Contributor Author

selaux commented Feb 18, 2019

Hi, yes that might work in this specific case (when there is one property). But my usecase is that either some-metadata-property is not set or it is an object with specific properties. Making all these properties optional will allow the user to set a partial object, which is not my goal.

selaux added a commit to selaux/netlify-cms that referenced this issue Feb 28, 2019
@maxgardner
Copy link
Contributor

I am running into this issue as well. On some collection pages, I have an optional primary field that has several sub-fields—some required and some optional—and I cannot save a new page because of the required sub-fields (only relevant if someone is going to include that primary field, which they don't have to), even though the primary field is optional.

@barthc
Copy link
Contributor

barthc commented Feb 28, 2019

@selaux @maxgardner technically the widget option required doesn't work on object and list widgets but on the actual input field widgets.

@selaux
Copy link
Contributor Author

selaux commented Mar 1, 2019

@barthc Is this intended or does it have historical reasons?

One way I could think of to improve this would be instead of setting empty values such as "", [] or {} for fields, we could just remove the field value altogether when it is empty. This way non-required objects would just not be set. I dont know if there would be other implications though.

Another way would be to reflect requiredness in the ui and allow to add and remove property values to/from objects manually (by some kind of +/- buttons to the object editor ui).

@barthc
Copy link
Contributor

barthc commented Mar 4, 2019

Is this intended or does it have historical reasons?

I think it makes sense this way cause there is a widget option pattern that works with the required option https://www.netlifycms.org/docs/widgets/#common-widget-options where you can define the regex pattern to validate against and the custom error message to display, again the pattern option technically doesn't work for list and object widget.

@erquhart
Copy link
Contributor

erquhart commented Mar 4, 2019

I'm honestly not certain what the original intent was, but the use cases mentioned here are sensible and should be supported. That said, and as @barthc points out, field validation may not be the right tool for the job.

List widget
On a list widget, what you really want is a set minimum number of items, with individual field validation being handled by the individual fields, including required validation.

Object widget
On an object widget, the UX of required validation would be confusing. All fields of the object are present and visible, and you can save the entry without filling them in, but if you fill in one of those fields, now the rest are required. There's no simple way to explain that to the user. What you really want is conditional visibility of a set of fields based on the value of another field, which again allows validation to be handled at the individual field level. This is covered under #1267.

Thoughts?

Sent with GitHawk

@maxgardner
Copy link
Contributor

@barthc That makes sense; thanks for explaining. The field validation definitely seems like it's working as expected in that case, and this particular problem is out of its scope.

@erquhart I think the conditional display of certain objects based on the user selecting to include that object would work well as a solution for my scenario; so long as the validation of any required sub-fields also was contingent upon this display status. Seems the issue you linked addresses that pretty clearly. Thank you!

@zakhap
Copy link

zakhap commented Oct 28, 2019

Is there any timeline on allowing object widget fields have the option "required: false"? This is a feature/bug problem I have run into inadvertently.

@brunoenribeiro
Copy link

brunoenribeiro commented Aug 2, 2022

I'm facing this issue right now. I'm not sure if the UX problem @erquhart explained is common, because I'm another user expecting that an optional field should pass validation if all its nested fields are empty. The current UX makes me expect this.

I'll explore solving this using #1267

@frederikcreemers
Copy link

Is there any progress on this? If not, I'd be willing to submit a PR that adds an "omit value" checkbox to the object widget, that collapses it and treats all fields inside as not required.

@martinjagodic
Copy link
Member

@bigblind this seems like a workaround. We are looking for objects to force their required: false to its child widgets. That should be done in the config, not having a UX component that allows that.

@frederikcreemers
Copy link

frederikcreemers commented Sep 25, 2024 via email

@martinjagodic
Copy link
Member

The original request is: don't validate the object if all fields are empty, but do validate if at least one field is not empty.

To achieve this, we don't need a new UI toggle, but we have to update the object validation to allow that.

@frederikcreemers
Copy link

Ok. I interpreted it as a more general request to allow omitting the values in optional object fields, didn't know the specific UX of just leaving fields blank was set in stone.

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

8 participants