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
[Alert] Use getContrastText for filled variant font color #29813
Changes from 1 commit
c54e076
f5f0aad
09881df
68d4fbb
a0aedd2
a04cdbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ const AlertRoot = styled(Paper, { | |
})(({ theme, ownerState }) => { | ||
const getColor = theme.palette.mode === 'light' ? darken : lighten; | ||
const getBackgroundColor = theme.palette.mode === 'light' ? lighten : darken; | ||
const getContrastText = theme.palette.mode === 'light' ? lighten : darken; | ||
mnajdova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const color = ownerState.color || ownerState.severity; | ||
|
||
return { | ||
|
@@ -71,7 +72,10 @@ const AlertRoot = styled(Paper, { | |
}), | ||
...(color && | ||
ownerState.variant === 'filled' && { | ||
color: '#fff', | ||
color: | ||
mnajdova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
theme.palette.mode === 'dark' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This (and line 46) relate to palette mode, but the value of contrast text should presumably depend on the background color being contrasted against, regardless of mode. The standard variant already has a light background with dark text, so I'm beginning to wonder if the premise of #29731 is faulty, although I suppose it might be the case that user provided colors used in the standard variant are light enough to justify dark text. |
||
? getContrastText(theme.palette[color].light) | ||
: getContrastText(theme.palette[color].light, 1), | ||
mnajdova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fontWeight: theme.typography.fontWeightMedium, | ||
backgroundColor: | ||
theme.palette.mode === 'dark' ? theme.palette[color].dark : theme.palette[color].main, | ||
|
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.
This returns the same as
getBackgroundColor
, so is redundant. It also doesn't do what its name suggests.We have
getContrastText
in the theme. @mnajdova is it also in System? I couldn't find it documented for either theme or system, but I believe it's what @reramjiawan was referring to in the related issue.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.
We only have it in the
theme.palette
as part of the@mui/material
's theme.