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

FIO-8185: Fixing issues with EditGrid and DataGrid clearOnHide with Conditionally visible elements. #77

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

travist
Copy link
Member

@travist travist commented Apr 9, 2024

Link to Jira Ticket

https://formio.atlassian.net/browse/FIO-8185

Description

We had an issue where conditionally visible components inside of EditGrid and DataGrid were getting cleared when they were submitted with the conditions available. What caused this problem is that the method "getComponentActualValue" takes the "component path" and not the "data path" so instead of being "editGrid[1].textField" it would come in as "editGrid.textField". Since we cannot modify the "editGrid.textField" string since that is saved during the building process, we needed to convert that path to the "data path".

This can be done by examining the parent component path, removing all [0] and then trimming the rest. This would also work with components that have complex keys such as "foo.bar".

I also had to make a change to the Logic component since another part of the issue is that we were using the "component.hidden" flag to clear the data when hidden. The problem with this is that it is not reverse compatible since there are a lot of other hidden fields with this flag set that are able to set the data on the server. Because of this, I had to make a change to the Logic processor so that it would set the scope.conditionallyHidden array to add the component that was hidden due to Logic forcing the component hidden.

Breaking Changes / Backwards Compatibility

It should be reverse compatible.

Dependencies

None

How has this PR been tested?

Automated tests were written.

Checklist:

  • I have completed the above PR template
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • My changes include tests that prove my fix is effective (or that my feature works as intended)
  • New and existing unit/integration tests pass locally with my changes
  • Any dependent changes have corresponding PRs that are listed above

@@ -20,7 +21,14 @@ export const clearHiddenProcess: ProcessorFnSync<ClearHiddenScope> = (context) =
if (!scope.clearHidden) {
scope.clearHidden = {};
}
if (component.hidden && value !== undefined && (!component.hasOwnProperty('clearOnHide') || component.clearOnHide)) {
const conditionallyHidden = (scope as ConditionsScope).conditionals?.find((cond) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now, but this type of type assertion makes me think we should just type the entire scope object as it is at runtime rather than kind of shifting the types around

@brendanbond brendanbond merged commit 3bf8538 into master Apr 9, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants