Skip to content
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

[TimePicker] Add Mui-selected class to TimeClock meridiem buttons #13848

Merged

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Jul 15, 2024

Fixes #12493.

Additional changes:

P.S. I picked this up after I saw an old stash with partially complete work. 🙈

@LukasTy LukasTy added component: pickers This is the name of the generic UI component, not the React module! component: TimePicker The React component. customization: css Design CSS customizability enhancement This is not a bug, nor a new feature labels Jul 15, 2024
@LukasTy LukasTy self-assigned this Jul 15, 2024
paddingLeft: 4,
paddingRight: 4,
width: CLOCK_HOUR_WIDTH,
[`&.${clockClasses.selected}`]: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this introduce any specificity change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point.
This could be a BC for someone currently overriding selected styles via theme.
I'll check.
As for the class specificity, I don't think we can run into it, because there was no way to identify the selected button based on DOM/Classes for now. 🙈

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If people are using the variants / the ownerState in their theme, would they see their CSS change because of specificity change on our side?
I think so but I'm not very good at CSS specificity 😆

Copy link
Member Author

@LukasTy LukasTy Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flaviendelangle
I've double-checked and indeed, we can't go this route, at least now, because for someone it would be a BC. 🙈
The following styles would no longer apply to a selected meridiem button:

MuiClock: {
  styleOverrides: {
    amButton: {
      color: 'red',
    },
    pmButton: {
      color: 'green',
    },
  },
},

Thus, I've reverted the change. 😉

@mui-bot
Copy link

mui-bot commented Jul 15, 2024

Deploy preview: https://deploy-preview-13848--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 00342db

@LukasTy LukasTy merged commit 923de76 into mui:master Jul 16, 2024
17 checks passed
@LukasTy LukasTy deleted the add-time-clock-meridiem-button-selected-class branch July 16, 2024 07:31
DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! component: TimePicker The React component. customization: css Design CSS customizability enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TimePicker] AM/PM button does not have selected class for font style customization
3 participants