fix: prevent posting invalid values #1069
Conversation
|
🚀 Deployed on https://pr-1069--dhis2-settings.netlify.app |
| } else { | ||
| // Update client-side state to invalid value | ||
| // to prevent the error UI from persisting when | ||
| // changing an invalid value back to the original value | ||
| settingsStore.state[key] = value | ||
| settingsStore.setState(settingsStore.state) | ||
| } |
There was a problem hiding this comment.
I found a bug after adding the first if clause.
- When the user changes the initial form value to an invalid value
- And then blurs the field
- The field will get and error state and show an error message (CORRECT)
- When the user focusses the field again
- And then changes the value back to the original value
- And then blurs the field
- The error message would persist
This was because the form state was not updated to the invalid value, so the state value was still the initial value. And as a result, changing the input's value back to initial state value was not triggering the onUpdateField callback.
Setting the state to the invalid value in the else clause makes sense, because the state should reflect the form-values whether valid or not. However, this does come at a price. The following steps will now produce a POST request, while strictly speaking they shouldn't:
- User changes initial value to an invalid value and blurs
- User changes invalid value to initial value and blurs
- The value is now different, so
onUpdateFieldis triggered. It is also valid, so the value is POSTed too.
Given the constraints of the tools being used here, I think this is probably as good as it will get.
There was a problem hiding this comment.
Probably good to point out that strictly speaking this is a "known issue", not a "regression". Since previously invalid values were being posted, resetting to an initial value would also trigger a post.
There was a problem hiding this comment.
Given the constraints of the tools being used here, I think this is probably as good as it will get.
agreed
| } else { | ||
| // Update client-side state to invalid value | ||
| // to prevent the error UI from persisting when | ||
| // changing an invalid value back to the original value | ||
| settingsStore.state[key] = value | ||
| settingsStore.setState(settingsStore.state) | ||
| } |
There was a problem hiding this comment.
Given the constraints of the tools being used here, I think this is probably as good as it will get.
agreed
Before this PR this app would unconditionally POST the new field value. Now this won't happen, which is obviously a very good thing. It seems that this improvement does come at a very small cost, which I attempt to explain in the code.