Skip to content
This repository has been archived by the owner on Feb 10, 2024. It is now read-only.

Possible typo in the number field validation code. #25

Open
eXtranium opened this issue Mar 9, 2023 · 2 comments
Open

Possible typo in the number field validation code. #25

eXtranium opened this issue Mar 9, 2023 · 2 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@eXtranium
Copy link
Contributor

As title says, I found a possible typo ("!" instead of "!!"), in the file:
fi.hg.frontend/blob/main/hooks/field/number/useFieldValidateNumberValueCallback.ts, line 30

In one project, when I clear the numbe input, the input field indicates missing value very shortly but returns to "it's ok" status. But if I add second exclamation mark there, the input field works correctly.

@eXtranium eXtranium added bug Something isn't working help wanted Extra attention is needed labels Mar 9, 2023
@eXtranium eXtranium self-assigned this Mar 9, 2023
@eXtranium eXtranium changed the title Possible typo on the number field validation code. Possible typo in the number field validation code. Mar 9, 2023
@eXtranium
Copy link
Contributor Author

The problem must be elsewhere, this was not a typo.

@thejhh
Copy link
Contributor

thejhh commented Mar 9, 2023

These states are some times hard to understand. It is also possible that the using side calls things incorrectly.

Let's open up some states:

  1. This function should return false if the field value is in incorrect state; and true if it is valid state
  2. The required=true means that the field value can be undefined

So this line should be return required ? true : false; and the code uses just an optimized version of this code.

It could help to add unit tests for this function in any case.

Most likely this is because something elsewhere is setting state to undefined initially and changing the state later. Probably correct fix is not to display the field and/or disable it until the field is ready. One way to handle this is also to add a loader over the whole form until it has loaded the data. We just added enabled property to some fields and should add same to the rest of fields as well. This could be used to make the field disabled until it has correct state loaded. The component nor the hook cannot know when it has been provided with correct state.

Another option could be to design a state for asynchronous operations which could indicate to the component that the initial data has not been loaded yet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants