search

guillotinaweb / ngx-schema-form

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

Issue with visibleIf in array items #479

Open Saeleas opened 12 months ago

Saeleas commented 12 months ago

Hello,

While doing an upgrade to the latest version (2.10.0) of ngx-schema-form from the version 2.8.4, we stumbled upon an issue.

The issue concerns the visibleIf property when it's on an array. If we define visibleIf in a property that is inside on an item of an array, this is not processed when it is created (after calling addItem), if the property exists in the model then the visibleIf property is processed correctly.

Here you will find an example on stackblitz: https://stackblitz.com/edit/angular-3b5bsy?file=src%2Fmain.ts chooseOne is used to decide whether to show a further string object or not. In the model myModel we setup an object and the visibleIf property gets updated correctly. If we use the add button then the visibleIf property is always shown whether we choose any option for chooseOne.

We believe that this change in behaviour was intruduced here: https://github.com/guillotinaweb/ngx-schema-form/pull/454 calling _bindVisibility() from ngOnChanges of form.component.ts instead of in createProperty in formpropertyfactory.ts

Was this change in behaviour intentional ? Or is it a undesired side effect ? If it was intentional, what would be the correct to define such property in our schema?

For the moment, in our implementation we had a custom component that implemented ArrayWidget, in this component we override addItem and there we call this.formProperty._bindVisibility(). This seems to solve this issue for the time being.

Thanks for your help!

ebrehault commented 12 months ago

That was not intentional. Please submit a pull request as you seem to have a fix.

Thank you!

Saeleas commented 12 months ago

Good to know! I (or maybe a colleague of mine) will make a pull request as soon as we can arrange some time to work on this.

KBroichhausen commented 12 months ago

Oops, I don't have dynamically added elements with visibleIf in my use case and sadly there is no test for that :( While I did this PR cause initially I had horrible cascading calls and load times of above 1 minute and was able to reduce it to like 4-5 seconds.

Maybe a solution would be to add a parameter to createProperty if it should directly bind visbility or not and which defaults to true? That potentially could fix all the custom widgets out in the wild doing something similiar. Basically, use only false in ngOnChanges for the rootProperty creation and pass it also for the recursive calls to createProperty in createProperty itself.

Saeleas commented 11 months ago

480