-
Notifications
You must be signed in to change notification settings - Fork 7
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
RN-1162: Fix visibility criteria check not updating when a answer changes #5402
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good approach - the tricky part of this is the different inputs that might come from ui-components
and what they return in theonChange
event. I have left a comment about autocomplete
questions, but other than that, pre-approving
if (updateFormDataOnChange) { | ||
setFormData({ | ||
[name]: e?.target ? e.target.value : e?.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there is a slightly weird case here, I think it's possibly in the autocomplete question and maybe also the entity question, where the onChange event returns an object with value
as opposed to a typical change event object, so we would need to handle that case as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexd-bes thanks for noting this, my original approach wasn't working with autocomplete questions as you predicted.
I decided to go with a slightly different approach this time. Now onChange
takes two arguments, the first is the input value and the second is the raw value. That way we can let react-form-hook
still do it's thing while also passing in the raw value to updateFormData()
034d1cc
to
5f3dff5
Compare
merge latest dev before testing
…ng a new option
merge latest dev before testing
Issue RN-1162:
Pretty simple bug. We assumed that the data coming through
onChange
as a change event. Reworked it so that we now should always return the actual new data when an input changes. This feels like the right approach, though I'm not sure if there's anything that we're mis-using/loosing fromreact-hook-form
here...