jsonforms icon indicating copy to clipboard operation
jsonforms copied to clipboard

Required property is not set if field is conditionally required

Open sasmangulyan-vineti opened this issue 5 years ago • 7 comments

Describe the bug When making the specific field required based on some condition the required property is still false.

To Reproduce Steps to reproduce the behavior:

  1. Schema contains something conditional { type: "object", properties: { b: { type: "boolean" }, c: { type: "string", minLength: 1 } }, if: { properties: { b: { enum: [false] } } }, then: { required: ["c"] } }
  2. When making the "b" from true to false (for example checking and unchecking the box), required property of ''c'' which has the renderer of MaterialInputControl will not be changed and will remain false.

Expected behavior After a conditional action the required property of MaterialInputControl to become true.

Browser (please complete the following information):

  • Chrome 74.0.3729.157 (Official Build) (64-bit)
  • Firefox 66.0.5 (64-bit)

Used Setup (please complete the following information):

  • react
  • material

sasmangulyan-vineti avatar May 31 '19 12:05 sasmangulyan-vineti

Thank you for your report. I just double checked, the if/then/else is working fine. We don't set the required property at all in the renderers. I adapted the title because of this.

We happily would accept a contribution for this.

eneufeld avatar Jun 01 '19 07:06 eneufeld

Hey @eneufeld @sdirix, I see this has been moved to the backlog, are there no plans to fix this soon? Having it in would be a big win for me 👍🏻

stevenmckinnon avatar Feb 17 '21 15:02 stevenmckinnon

@stevenmckinnon In general I think setting the required prop as suggested by you in #1695 is a good idea and we should implement it in one of the next versions (i.e. next), however it's not on our priority shortlist. You're welcome to open a PR to fix the issue, and we'll definitely have a look at that.

The requested feature here relates to if/then/else and will be much harder to support. As the use case is rather niche we have no plans to support this in the near future. If you'd like to contribute this feature we should discuss how the optional required can be detected and evaluated efficiently.

In case you need a solution for right now you can always just write your own custom renderers which set the required prop appropriately.

You might also be interested in our support packages which includes prioritization and development of requested features.

sdirix avatar Feb 17 '21 16:02 sdirix

Good to know, thanks @sdirix!

stevenmckinnon avatar Feb 17 '21 16:02 stevenmckinnon

Hey there, we stumbled upon the exact same issue of properties becoming required via some conditions. Whether that is directly in the "root" schema or in some nested anyOf|oneOf|allOf section. And unfortunately the renderers property control.required do not get updated if the conditions are met. I think it is a rather common use case and not that much of a niche. Imo it is common to e.g. toggle additional form fields via a checkbox which then become required.

I highly appreciate this feature being supported.

To find out whether a component shall become required or not, if it just got mounted because of some layout condition which is now met, we came up with the following workaround:

mounted() {
    const validate = this.$validate(this.ajv, this.jsonforms.core.data, this.jsonforms.core.schema);
    this.isRequired = validate.errors.some(({ keyword, params }) => {
        return keyword === 'required' && params.missingProperty === this.control.path;
    });
}

nmaier95 avatar Jul 21 '22 14:07 nmaier95

Hi @nmaier95,

I don't see this feature implemented generically soon. We can't really reuse AJV as in the example code as then isRequired is only true when the parameter is missing. However it should also be true when it's there and there is no validation error.

We would need to evaluate the if conditions of AJV schemas for if/then/else cases manually. In bad cases the structure for this could be defined completely in parallel to the "normal" schema. Also for oneOf and anyOf it's completely unclear how a UI should behave.

If you need something like this right now then you probably have some very specific structures you need to see supported. For now I would like to suggest to implement the support for them on your own.

Of course we're open for a generic solution but it should be something without too much of a runtime penalty for this "small" feature.

sdirix avatar Jul 24 '22 22:07 sdirix

Hey there, we stumbled upon the exact same issue of properties becoming required via some conditions. Whether that is directly in the "root" schema or in some nested anyOf|oneOf|allOf section. And unfortunately the renderers property control.required do not get updated if the conditions are met. I think it is a rather common use case and not that much of a niche. Imo it is common to e.g. toggle additional form fields via a checkbox which then become required.

I highly appreciate this feature being supported.

To find out whether a component shall become required or not, if it just got mounted because of some layout condition which is now met, we came up with the following workaround:

mounted() {
    const validate = this.$validate(this.ajv, this.jsonforms.core.data, this.jsonforms.core.schema);
    this.isRequired = validate.errors.some(({ keyword, params }) => {
        return keyword === 'required' && params.missingProperty === this.control.path;
    });
}

This won't work. When you fill field property the required property will be set to false since there will be no more errors.

IMO the problem is with isRequired function in core/renderers file:

const isRequired = (
  schema: JsonSchema,
  schemaPath: string,
  rootSchema: JsonSchema
): boolean => {
  const pathSegments = schemaPath.split('/');
  const lastSegment = pathSegments[pathSegments.length - 1];
  // Skip "properties", "items" etc. to resolve the parent
  const nextHigherSchemaSegments = pathSegments.slice(
    0,
    pathSegments.length - 2
  );
  const nextHigherSchemaPath = nextHigherSchemaSegments.join('/');
  const nextHigherSchema = Resolve.schema(
    schema,
    nextHigherSchemaPath,
    rootSchema
  );

  return (
    nextHigherSchema !== undefined &&
    nextHigherSchema.required !== undefined &&
    nextHigherSchema.required.indexOf(lastSegment) !== -1
  );
};

It just search for up/down schema. Not checked at all allOf anyOf or flat if statement in schema

wojtek1150 avatar May 30 '23 07:05 wojtek1150