-
Notifications
You must be signed in to change notification settings - Fork 6
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
FIO-8414: Fix required validation not working in Data Grid #105
FIO-8414: Fix required validation not working in Data Grid #105
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.
LGTM but why no tests?
Oh, I see your comment re: formio.js cool cool - I think it would be good to duplicate those though |
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.
Can we discuss in dev support? At first glance, I'm concerned that the core library shouldn't really be dealing with instance
in any meaningful way, we need to derive these types of state from the JSON if at all possible. But I also could be misunderstanding how your solution fits together, so if you could take me through it that would be great!
return false; | ||
} | ||
|
||
const isArrayDataComponent = dataValue.every(_isPlainObject); |
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.
Would it be possible to use the existing getModelType
family of functions here?
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.
@brendanbond would be problematic to use the mentioned function here, since a couple of upper functions recieve only 'value' from context, but in order to use this func, we need a 'component'.
Do you think it's worth changing upper functions to recieve component/whole context object?
I'm having trouble actually reproducing this issue. @mikekotikov I'm going to wait on this until we review this w/ the QA folks and see if this problem has changed or been resolved |
…d_datagrid FIO-8414: Fix required validation not working in Data Grid
Link to Jira Ticket
https://formio.atlassian.net/browse/FIO-8414
Description
What changed?
Added check for array data values in required validation
Why have you chosen this solution?
Use this section to justify your choices
Breaking Changes / Backwards Compatibility
Use this section to describe any potentially breaking changes this PR introduces or any effects this PR might have on backwards compatibility
Dependencies
@formio/js:5646
How has this PR been tested?
Automated tests are added in @formio/js PR
Checklist: