-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Select] Make error part of the ownerState
to enable overriding styles with it in theme
#36422
Conversation
… a custom theme for MuiSelect Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Netlify deploy previewhttps://deploy-preview-36422--material-ui.netlify.app/ Bundle size report |
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@siriwatknp this PR is ready for review |
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.
Select with error
prop has a border color applied for error state. See https://codesandbox.io/s/falling-leaf-v6uuzt?file=/src/App.tsx.
While it does not work after changes in this PR, see https://codesandbox.io/s/material-cra-ts-forked-m06iu4?file=/src/App.tsx.
ownerState
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Thanks for the review @ZeeshanTamboli, your feedback comment has been addressed. |
packages/mui-material/src/NativeSelect/NativeSelectInput.test.js
Outdated
Show resolved
Hide resolved
ownerState
ownerState
to enable overriding styles with it in theme
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.
Left some comments. Can you also add a test for native select? Like a Select
component with the native
prop (<Select native/>
).
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Your feedback comments have been addressed @ZeeshanTamboli, please check again. |
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
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.
Few nits!
packages/mui-material/src/NativeSelect/NativeSelectInput.test.js
Outdated
Show resolved
Hide resolved
packages/mui-material/src/NativeSelect/NativeSelectInput.test.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com>
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com>
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com>
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com>
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.
@gitstart Looks great! Thanks!
@ZeeshanTamboli Thanks for the review rounds. |
closes #36269
This code was written and reviewed by GitStart Community. Growing future engineers, one PR at a time.