-
Notifications
You must be signed in to change notification settings - Fork 1.1k
FIO-9974 fixed Clear on Hide for layout components #6112
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
base: master_old
Are you sure you want to change the base?
FIO-9974 fixed Clear on Hide for layout components #6112
Conversation
travist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There has to be a better way to do this other than having to iterate through ALL your children in the deleteValue method. My guess is this will cause performance regressions.
|
The approach was changed and now the parent layout-component check is used instead of the eachComponent method. |
|
@travist can you review the latest changes to this pr |
| hasCondionallyHiddenLayoutParent() { | ||
| let currentParent = this.parent; | ||
| while (currentParent) { | ||
| if (currentParent._conditionallyHidden && FormioUtils.isLayoutComponent(currentParent) && currentParent.component.clearOnHide === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If "clearOnHide" is not set, I believe it assumes it is "true". This will not work for the case when the clearOnHide value is not set (undefined).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@travist If ClearOnHide is not set, then the corresponding value is set from the component schema. For Layout components, this value is false (example Panel component). Therefore, according to the documentation, if the clearOnHide property is not set for the Layout of components, then the nest component field values will not be cleared.
I also added tests for both possible options: if clearOnHide is set and not set.
Link to Jira Ticket
https://formio.atlassian.net/browse/FIO-9974
Description
According to the documentation, for Layout components, such as Panels, Well, Fieldset, etc the default value of
clearOnHideis set tofalse. That is, when hiding the parent Layout component , the value of the child components should not be cleared (see Example Scenarios 3). But the value of theclearOnHideproperty can be manually overwritten totrueusing the Layout component's JSON schema. Several issues related to this functionality have been found and fixed:clearOnHideproperty was set totrue. This means that when hiding the FieldSet component, all the values of the child components were always cleared, regardless of whetherclearOnHideproperty was set in the JSON schema or not, contrary to the documentation. This was fixed by addingclearOnHideproperty asfalsefor the FieldSet component a default value.clearOnHideproperty astruedid not change the behavior of the Layout components. That is, even if the User manually set theclearOnHideproperty totruein the Layout component's JSON schema, the values of the child components were not cleared, contrary to the documentation. This was due to the fact that Layout components, such as Panels, Well, Fieldset, etc do not have their own values, therefore, in thedeleteValuemethod, the values of child components were ignored and not cleared, even when settingclearOnHideastruein the JSON schema of the Layout component . This was fixed by adding a check for Layout components and clearOnHide property.Breaking Changes / Backwards Compatibility
n/a
Dependencies
n/a
How has this PR been tested?
Added automatic tests covering the behavior of the clearOnHide functionality for the Layout component for both cases: default behavior and manual overwriting of the clearOnHide property as true. All tests pass locally
Checklist: