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

[DynamicForm] - Number validation is preventing form save in certain circumstances, not enabled for currency fields #1604

Closed
t0mgerman opened this issue Jul 31, 2023 · 1 comment · Fixed by #1709
Labels
status:fixed-next-drop Issue will be fixed in upcoming release. type:bug
Milestone

Comments

@t0mgerman
Copy link

Category

[ ] Enhancement

[x ] Bug

[ ] Question

Version

Please specify what version of the library you are using: [ v3.15 ]

Expected / Desired Behavior / Question

I've identified two issues:

Firstly, number validation added in 8998b5d seems to have broken form validation where:

  • a numeric list field is set as not required
  • that numeric list field has a minimum and maximum value set
  • the user chooses not to enter a value, or enters a value then deletes it

I had a some lists where fields had minimum and maximum values between 1 and 5 - and they all stopped working in DynamicForm after recently updating the library in one of my projects.

Secondly, the validation added does not yet work for Currency fields.

Observed Behavior

The form silently fails to save if the user leaves the field blank. The reason this happens is because the check in DynamicForm.tsx sets shouldBeReturnBack to true if val.newValue < val.minimumValue. If the user leaves a field empty, val.newValue is an empty string "" - which is falsey, and equivalent to zero. So if the minimum value is, for example, 1 - then shouldBeReturnBack is set to true, and the form doesn't save. For a list where the field is not required, this is incorrect behaviour. In DynamicField.tsx the logic that determines the error to display DOES check the length of the string value first, and so - no error is returned to tell the user why the form didn't save.

Steps to Reproduce

  • Add a number column to a list
  • Set the minimum and maximum values to 1 and 5 respectively.
  • Using DynamicForm, enter and then delete a value from this field and attempt to save a form while this value is blank.

For the currency issue, observe that validation added in the referenced commit does not also work for those fields.

I have a PR ready to go for the above.

@ghost
Copy link

ghost commented Jul 31, 2023

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

@ghost ghost added the Needs: Triage 🔍 label Jul 31, 2023
@michaelmaillot michaelmaillot added status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. type:bug and removed Needs: Triage 🔍 labels Sep 13, 2023
@AJIXuMuK AJIXuMuK added status:fixed-next-drop Issue will be fixed in upcoming release. and removed status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. labels Oct 8, 2023
@AJIXuMuK AJIXuMuK added this to the 3.16.0 milestone Oct 8, 2023
@AJIXuMuK AJIXuMuK mentioned this issue Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:fixed-next-drop Issue will be fixed in upcoming release. type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants