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

Issue with visibleIf in array items #479

Open
Saeleas opened this issue Jul 24, 2023 · 4 comments
Open

Issue with visibleIf in array items #479

Saeleas opened this issue Jul 24, 2023 · 4 comments

Comments

@Saeleas
Copy link

Saeleas commented Jul 24, 2023

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: #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
Copy link
Collaborator

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

Thank you!

@Saeleas
Copy link
Author

Saeleas commented Jul 24, 2023

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
Copy link
Contributor

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
Copy link
Author

Saeleas commented Aug 9, 2023

#480

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

No branches or pull requests

3 participants