-
Notifications
You must be signed in to change notification settings - Fork 373
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
Add required prop to Inputs #1798
Conversation
5a82ded
to
399234c
Compare
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 I think this can also be applied to MaterialNativeControl
https://github.com/eclipsesource/jsonforms/blob/master/packages/material/src/controls/MaterialNativeControl.tsx#L77-L81
399234c
to
2726602
Compare
Thank You! I updated the pull request and also included the MaterialNativeControl. |
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, thanks 🎉
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.
Works for me! Good work!
I only have some minor comments.
2726602
to
4b15f48
Compare
I applied your suggested changes |
closes eclipsesource#1695 Signed-off-by: Lukas Boll <lukas-bool@web.de>
4b15f48
to
3f6e587
Compare
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
Material-Ui components are now using the required prop where possible.
closes #1695
Signed-off-by: Lukas Boll lukas-bool@web.de