-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[Tabs] Simplify override #14638
[Tabs] Simplify override #14638
Conversation
@material-ui/core: parsed: -0.18%:heart_eyes:, gzip: -0.11%:heart_eyes: Details of bundle changes.Comparing: cbed923...536ef7b
|
dec2076
to
bfb0760
Compare
marginRight: 30, | ||
marginLeft: 30, | ||
}, | ||
[attach(TAB.selected)]: { |
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.
Hi @siriwatknp, If you have some time, I think that it would be great that we take the time to find the best possible theme implementation. We have different theme demos, the approach isn't consistent. I think that we should try to unify them.
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.
Hi @oliviertassinari, should we open a discussion topic about mui-theming?
66f0a4b
to
4a2211c
Compare
4a2211c
to
7603f8c
Compare
7603f8c
to
536ef7b
Compare
Better than 0.011% I guess, but prbot might be being a little over-enthusiastic here 😄 |
[nest(ICON.root)]: { | ||
minHeight: null, | ||
paddingTop: null, | ||
'& $wrapper :first-child': { |
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.
Oh this rule applies to all first-childs in the tree. This messes things up, if you are using icons and labels.
'& $wrapper :first-child' === '& $wrapper *:first-child'
I think you want to omit the space here:
'& $wrapper:first-child'
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.
In my case I am using Tabs with Labels, Icons and Badges. The above rule is applied to all of them.
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.
Are you using this custom theme? Yeah, I'm happy to try the change.
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.
Oh actually not. I added this comment to the wrong part of the code 😬. Nevertheless the issue stays the same. You might want look at this, too.
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.
Here is an example: https://codesandbox.io/s/53o696672p
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.
What about?
'& $wrapper > *:first-child': {
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.
testes locally & lgtm! 😍
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.
Awesome! Do you want to submit a pull request? :)
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.
Happy to!
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.
Done #14844
Introduce a new
wrapped
property. Closes [Tabs] Awkward line height when 2 lines #14538Add a new customization demo. Handle Tabs indicator custom width and position #10465 (comment).
Breaking change
We have removed the
labelContainer
,label
andlabelWrapped
class keys. We have removed 2 intermediary DOM elements. You should be able to move the custom styles to the root class key.