guillotinaweb / ngx-schema-form

HTML form generation based on JSON Schema
MIT License
484 stars 173 forks source link

Required nested properties valid even though they are empty #313

Open MyleneSimon opened 5 years ago

MyleneSimon commented 5 years ago

We are using the library to generate forms for tasks configured by users as part of scientific workflows. Our schemas usually consist of a taskName property (string), and a parameters property (object with nested properties), for example:

{ "properties": {
      "taskName": {
        "type": "string",
        "title": "Task name",
        "description": "(required)",
        "placeholder": "Enter a name for this task"
      },
      "parameters": {
        "type": "object",
        "description": "Parameters",
        "properties": {
          "paramA": {
            "type": "string",
            "title": "Param A",
            "description": "(required)"
          },
          "paramB": {
            "type": "string",
            "title": "Param B",
            "description": "(not required)"
          }
        },
        "required": ["paramA"]
      }
    },
    "required": ["taskName", "parameters"]
}

In this case, top level property taskName is required, as is nested property paramA. Our issue is that required nested properties have the class has-success, with the input having the class ng-valid even though they are empty. But validation seems to be working, since errors for missing property paramA are displayed. This prevents us from using the has-error/has-success or ng-valid/ng-invalid classes to inform the user about which fields are valid and which are not. On the other hand, top level property taskName has the class 'has-error", with the input having the class ng-invalid as expected when empty.

Stackblitz reproducing the issue

Are we missing something here, or is it a bug?

daniele-pecora commented 4 years ago

As I see, empty fields are not handled as invalid inputs if they are required.
An exception is made for 1st level children. For all nested fields the parent container is set to ng-invalid instead.

See this example here based on yours: https://stackblitz.com/edit/ngx-schema-form-customs-r1nkga-313

It takes also the attribute required in count to style invalid fields

MyleneSimon commented 4 years ago

So this is the intended behavior then? What puzzles me a bit is that if one of the other nested properties is not empty (for example, typing something for Param B), then the validation errors will show "Missing required property: paramA", and yet the field will be in valid state. The issue is that if we base our styling on the has-error of the parent, then all nested fields will appear as invalid until all of them are valid, which can be confusing for the user, because we often have a lot of properties nested under our parameters property. Are there any suggested workarounds for that?

daniele-pecora commented 4 years ago

So this is the intended behavior then? What puzzles me a bit is that if one of the other nested properties is not empty (for example, typing something for Param B), then the validation errors will show "Missing required property: paramA", and yet the field will be in valid state.

It's a kind of yes and no (Schrödinger). Yes, the error shows up but no it isn't referenced to the field rather to the parent container.

I've modified the example at Stackblitz a bit so it shows the path of the control where the error comes from. Also it is now using the :invalid pseudo selector to address required field.

Bildschirmfoto 2019-11-20 um 19 01 13

As you see the validator doesn't apply the error to the required child but to the parent control parameters.

This must rely in the json schema specification, where a field doesn't declare any property required itself (btw earlier version of json schema did this but then changed), but the parent declares all required children.

Simplified version:
A field is only invalid if a pattern or other kind of restriction doesn't apply.
If a field is required, then the parent is invalid but not the field.
To get an empty fiel invalid you must specify some restrictions like minLength, maxLength, pattern etc...

MyleneSimon commented 4 years ago

@daniele-pecora thank you for the explanation, using the :invalid pseudo selector might just do the trick for us!

develAman commented 4 years ago

Needed to check validness of fields/nested fields in a fieldset, to show status for each step of the stepper Here is an example code in stackblitz.com

Some required fields are working as not required (lastName, transactionNumber in step 1, category in step 2, and password in step 3) and some not required fields are working as required (favoriteAnimal select in step 2)

Also was having problem similar as here #313 with required fields, fixed control itself with next workarround for string widget:

    ngAfterViewInit() {
        super.ngAfterViewInit();

        if (this.schema.isRequired && !this.schema.minLength) {
            this.control = _.clone(this.control);
        }
    }

but it doesn't fix formProperty,

-To get an empty fiel invalid you must specify some restrictions like minLength, maxLength, pattern etc... -but that is ok maybe for small schemas

-If a field is required, then the parent is invalid but not the field. - but parent can have many other not relevant invalid fields

Question 1: would be nice to have required working as expected, so how is it possible?

Question 2: minLength is working as combination of minLength and required now, but what if a field is not required and needs to be set minLength?

daniele-pecora commented 4 years ago

Question 1: would be nice to have required working as expected, so how is it possible?

Answer Create a custom schema validator that corrects the error message path, so the error is mapped to the field itself. Working sample: custom-zschema-validator-factory.ts

Question 2: minLength is working as combination of minLength and required now, but what if a field is not required and needs to be set minLength?

Answer Create a custom schema validator that ignores the error if the field is not required. Working sample: fix-optional-empty-fields-z-schema-validator-factory.ts

Usage working sample https://github.com/daniele-pecora/ngx-schema-form-widgets-material

develAman commented 4 years ago

Thank you @daniele-pecora for your quick answer, will try to customize the validator. Nice working sample btw, I will use them in the future :)