-
-
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
[DateTimePicker] Fix not visible selected tab icon #26624
Conversation
@nikitabobers It looks already much better. I have tried something else
What do you think? the am/pm toggle so far away from one another seems plain wrong, but that's a different topic. |
packages/material-ui-lab/src/DateTimePicker/DateTimePickerTabs.tsx
Outdated
Show resolved
Hide resolved
packages/material-ui-lab/src/DateTimePicker/DateTimePickerTabs.tsx
Outdated
Show resolved
Hide resolved
packages/material-ui-lab/src/DateTimePicker/DateTimePickerTabs.tsx
Outdated
Show resolved
Hide resolved
@oliviertassinari I agree. Now it's even better and easy for user to see which Tab are selected. I'll to commit that changes. For AM/PM buttons should we open another issue ? |
@nikitabobers Yes, it's off-topic to these changes. As for opening another issue, since the time picker is 1/5 of the use case for the date picker, I would say no real need, we might want to focus more on the date picker. |
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 have pushed the solution the extra mile. You can see one commit per improvement.
@nikitabobers it's a great first pull request on Material-ui 👌 Thank you for working on it! |
Thank you! |
Closes #26283
Preview: https://deploy-preview-26624--material-ui.netlify.app/components/date-time-picker/#basic-usage
My suggestion:
Let me know what dou you think about it 😉