Skip to content

[Typography] Fix styleOverrides, variants, and remaining colors #34444

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: v5.x
Choose a base branch
from
Open

[Typography] Fix styleOverrides, variants, and remaining colors #34444

wants to merge 3 commits into from
+76 −5

Conversation

@emillaine
Copy link
Contributor

@emillaine emillaine commented Sep 23, 2022 •
edited
Loading

Fixes #34290.
Fixes #29564.

TODO:

  • Fix extra generated CSS rule color: textPrimary.

@mui-bot
Copy link

mui-bot commented Sep 23, 2022 •
edited
Loading

Details of bundle changes

@material-ui/core: parsed: +0.04% , gzip: +0.09%

Generated by 🚫 dangerJS against d3284d8

@emillaine
Typography: fix styleOverrides and variants to work
7152cf7 
They were being overridden by the system props.
@emillaine emillaine changed the title [Typography] Fix styleOverrides not overriding system props [Typography] Fix styleOverrides and variants to work Sep 23, 2022
@emillaine emillaine force-pushed the fix-typography-color-override branch from cec28f4 to 7152cf7 Compare September 23, 2022 14:48
@emillaine emillaine marked this pull request as ready for review September 23, 2022 14:48
@emillaine emillaine changed the title [Typography] Fix styleOverrides and variants to work [Typography] Fix styleOverrides, variants, and remaining colors Sep 23, 2022
@emillaine emillaine mentioned this pull request Sep 23, 2022
Closed
@ZeeshanTamboli ZeeshanTamboli added type: bug It doesn't behave as expected. scope: typography Changes related to typography. labels Sep 24, 2022
@emillaine
Copy link
Contributor Author

emillaine commented Oct 21, 2022 •
edited
Loading

The CI yarn proptypes check complains about missing color prop, but that seems wrong since this PR didn't add a color prop.

@madroneropaulo
Copy link

madroneropaulo commented Dec 12, 2022

Any updates on this?

@emillaine
Copy link
Contributor Author

emillaine commented Dec 16, 2022

Nope. The CI test fails for some reason I don't understand. On top of that, my team is working on a major version update to our MUI-based library, which allows us to fix the problem that prompted #34290 by modifying the palette, removing the need for styleOverrides or other workarounds.

So this PR is currently not prioritized for us. Feel free to pick it up though :)

@michaldudak michaldudak removed their request for review January 4, 2023 14:27
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 3, 2023
@Londeren
Copy link

Londeren commented Mar 1, 2024

Guys, any news here?

siriwatknp
siriwatknp reviewed Mar 5, 2024
View reviewed changes
packages/mui-material/src/Typography/Typography.js
Comment on lines +140 to +143
sx={[
...(!Object.keys(colorTransformations).includes(color) ? [{ color }] : []),
...(Array.isArray(sx) ? sx : [sx]),
]}
Copy link
Member

@siriwatknp siriwatknp Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This makes me think that there must be something wrong with the Typography code wise.

Copy link
Member

@siriwatknp siriwatknp Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd say that we should look into this in v7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siriwatknp siriwatknp siriwatknp left review comments

+1 more reviewer

@xepozz xepozz xepozz approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

PR: out-of-date The pull request has merge conflicts and can't be merged. scope: typography Changes related to typography. type: bug It doesn't behave as expected.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Typography] Cannot customize primary/error color Typography does not support color "success"

7 participants

@emillaine @mui-bot @madroneropaulo @Londeren @xepozz @siriwatknp @ZeeshanTamboli