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

FE logic: When the value of number field is used to trigger visibility, components are emptied when "Clear on hide" is checked #3964

Closed
LaurensBurger opened this issue Mar 1, 2024 · 7 comments · Fixed by open-formulieren/formio-builder#136 or #4183
Assignees
Labels
bug Something isn't working needs-backport Fix must be backported to stable release branch
Milestone

Comments

@LaurensBurger
Copy link
Collaborator

LaurensBurger commented Mar 1, 2024

Product versie / Product version

2.4.3 - 2.6.0

Omschrijf het probleem / Describe the bug

When the value of number/currency field is used in FE logic and the component that is made visible has: "Clear on hide" checked, the values entered into this field will be removed at validation (check_logic)

if Getal = 0 show Tekstvlak

pooof
Kooha-2024-03-01-21-23-53

If a different logic is used on the same Number component like: if not Getal hide Tekstvlak
This bug does not occur, so it seems to be specific to the value in the number field

Discussion:

We changed the formio-builder so that for new forms the conditional value is not saved as a string but as a number.

However, this will not fix existing forms that have a configuration where the conditional value is already a string. For fixing these forms there were 2 possibilities:

  • Migration: find forms that have this problem and fix the configuration. This migration is quite complex and would also have to run on import.
  • Bandaid fix in the logic (like for checkboxes:
    # Issue #2900 - The values of checkboxes are bools, but in the Formio frontend logic, the compare value is a
    ). This is quite easy, although care needs to be taken with the int/float casting.

Considerations:

  • Writing the migration feels like the 'right thing': new components have fixed behaviour and you don't want to carry along a legacy fix forever. It's better to root it out as soon as possible.
  • Backporting the migration is tricky

Conclusion:

  • Migration for master
  • Bandaid for backport

In addition:

  • See if you can fix checkboxes so we can remove the bandaid fix in master
@LaurensBurger LaurensBurger added bug Something isn't working triage Issue needs to be validated. Remove this label if the issue considered valid. labels Mar 1, 2024
@joeribekker joeribekker removed the triage Issue needs to be validated. Remove this label if the issue considered valid. label Mar 4, 2024
@SilviaAmAm
Copy link
Contributor

SilviaAmAm commented Apr 4, 2024

This is the same problem as here: #2900
Except now with numbers instead of bools. The frontend conditions are saved in the configuration as:

"conditional": {
   "eq": "0", 
   "show": true, 
   "when": "number"
}

(note "0"!)
However, when checking the logic, the value sent to the backend for the number/currency components is of number type. The backend interprets "0" as different from 0 and therefore the value of the text area component is cleared.

@sergei-maertens
Copy link
Member

So this can be worked around by editing the JSON? I'm trying to figure out of this needs backporting.

And a patch would be in the form builder to cast the value of eq to the respective type of the referenced component in when?

@SilviaAmAm
Copy link
Contributor

Yes, if the "0" is changed in the json to 0, it works as expected.

I was wondering if in the form builder, the field for the conditional value should have the same type as the component selected:
matching-type

Then there would be no need to cast I think?

@sergei-maertens
Copy link
Member

The casting logic should be done based on the component type, since the component type determines the data type, or am I misunderstanding something?

The option labels are {component.label} ({component.key}) here, the value between brackets is not the type of component :) Perhaps that is causing some confusion?

@SilviaAmAm
Copy link
Contributor

What I meant is:

If you select a component of type number in the dropdown, then the input field below (field with label has the value) would be:

<input type="number" ... >

instead of a text field. So the type of the input would depend on the component selected for the condition. That way it would have the right type of value without casting.

@sergei-maertens
Copy link
Member

Ah right, that makes sense! Note however that it's Formik that takes care of casting the string values to numbers then, and this would need to apply to component types number and currency both, but yes, your approach is much more elegant :)

@SilviaAmAm
Copy link
Contributor

I thought a bit more about it:

We can't really rely only on the type of the chosen component (if the component has multiple = True then it is an array instead of whatever other simple type). So we would need a field like the DefaultValue react component that each component has in the edit form.

However, this would break the logic check for the selectboxes. These have a value that is an object, but in the 'simple conditional' configuration you enter a string. Then, Formio does some special checks when evaluating the logic. For example, if the value is {"valueA": true, "valueB": false} and the conditional is:

"conditional": {
   "eq": "valueB", 
   "show": true, 
   "when": "selectboxesComponent"
}

then formio does some special check to see if valueB == True.

So more thinking needed 💭

SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 5, 2024
…field

The simple conditional input field now depends on the selected component
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 8, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 8, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 8, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 8, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 8, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 11, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 11, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 11, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 11, 2024
SilviaAmAm added a commit to open-formulieren/formio-builder that referenced this issue Apr 11, 2024
sergei-maertens pushed a commit to open-formulieren/formio-builder that referenced this issue Apr 12, 2024
sergei-maertens pushed a commit to open-formulieren/formio-builder that referenced this issue Apr 12, 2024
sergei-maertens pushed a commit to open-formulieren/formio-builder that referenced this issue Apr 26, 2024
sergei-maertens pushed a commit to open-formulieren/formio-builder that referenced this issue Apr 26, 2024
sergei-maertens added a commit to open-formulieren/formio-builder that referenced this issue Apr 26, 2024
sergei-maertens added a commit that referenced this issue Apr 26, 2024
sergei-maertens added a commit that referenced this issue Apr 26, 2024
sergei-maertens pushed a commit that referenced this issue Apr 26, 2024
sergei-maertens pushed a commit that referenced this issue Apr 26, 2024
SilviaAmAm added a commit that referenced this issue Apr 29, 2024
SilviaAmAm added a commit that referenced this issue Apr 29, 2024
SilviaAmAm added a commit that referenced this issue Apr 29, 2024
sergei-maertens added a commit that referenced this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment