[CssBaseline] Add colors with better constrast to <a> #38135

brijeshb42 · 2023-07-24T09:48:45Z

in CssBaseline component for both light and dark modes

Fixes #38017

I have followed (at least) the PR section of the contributing guide.

Light

Dark

brijeshb42 · 2023-07-24T09:49:05Z

@danilo-leal Should this also be done for Joy UI?

mui-bot · 2023-07-24T09:51:28Z

Netlify deploy preview

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against fa0a76a

zanivan · 2023-07-24T15:18:45Z

Hey @brijeshb42 thanks for tackling this!

Should this also be done for Joy UI?

If the same thing is happening on Joy UI, I see no reason not to solve it there as well

brijeshb42 · 2023-07-24T15:55:06Z

Do you want the same tokens there as well or different?

zanivan · 2023-07-24T16:03:15Z

Is it possible to use the Joy default theme? If so, you can use the primary 500 #0B6BCB for light and primary 300 #97C3F0 for dark

brijeshb42 · 2023-07-24T16:54:45Z

@zanivan I don't see the colors you've mentioned in the default theme. These are the primary tokens in Joy's default theme - https://github.com/mui/material-ui/blob/master/packages/mui-joy/src/colors/colors.ts#L14

zanivan · 2023-07-24T17:03:35Z

Right, sorry, my mistake 😅
I sent the colors that we're using on the new default theme #36843
That said, you can keep the same primary 500 and primary 300, but with the current token colors—and when we merge the new default theme it can be revamped.

siriwatknp · 2023-07-25T01:36:33Z

packages/mui-material/src/CssBaseline/CssBaseline.js

@@ -43,6 +43,10 @@ export const styles = (theme, enableColorScheme = false) => {
    'strong, b': {
      fontWeight: theme.typography.fontWeightBold,
    },
+    a: {
+      color:


Suggested change

color:

color: (theme.vars || theme).palette.primary.main,

I think primary.main should be the best one for links?

I've followed @danilo-leal's pointer #38017 (comment)

@siriwatknp Do you still want the change to reference theme.vars ?

It might be better as identical to what Link uses for design consistency

https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Link/Link.js (color + text decoration + hover)

in CssBaseline component for both light and dark modes

docs/data/material/components/css-baseline/css-baseline.md

oliviertassinari · 2023-07-27T20:07:16Z

Is this even needed? I would expect enableColorScheme to cover the color contrast bug report in #38017.

Now, it makes sense to me to consider if we want design consistency from CssBaseline, I would expect Tailwind CSS and Bootstrap to reset the look of links as well. For v6, I wanted to revisit all the style in CssBaseline, e.g. https://moderncss.dev/modern-css-for-dynamic-component-based-architecture/. I mentioned it in #30660

brijeshb42 · 2023-07-31T10:11:09Z

@oliviertassinari This is mainly added so that the link colors are set to material/joy colors for both dark/light mode. Reset from tailwind or other solution don't do anything for the color.

danilo-leal

Looking good! The colors I provided earlier were pulled from the company's branding theme, but it does sound like a good call to use each package's default theme colors, same tokens though. In summary:

Material UI uses Material Design colors.
Joy UI uses the Joy default theme colors.
Continue to use 500 for light and 300 for dark mode.

docs/data/joy/components/css-baseline/css-baseline.md

docs/data/material/components/css-baseline/css-baseline.md

oliviertassinari · 2023-07-31T12:55:45Z

Reset from tailwind

@brijeshb42 I meant this https://github.com/tailwindlabs/tailwindcss-typography/blob/master/src/styles.js#L1245-L1249.

oliviertassinari · 2023-08-01T19:21:05Z

The more we add style resets like this, the more I think in the future we can consider isolating each reset type:

One for pure default CSS coherence and ease, e.g. https://tailwindcss.com/docs/preflight
One for theme design token applications, e.g. https://tailwindcss.com/docs/typography-plugin

In our case, maybe we shouldn't reset the <a> link style in CssBaseline, but to keep it focused on 1. We could cover the issue with the <a> link style under 2. with #14876. I don't know but to consider.

packages/mui-joy/src/CssBaseline/CssBaseline.tsx

mnajdova · 2023-11-14T12:26:52Z

packages/mui-material/src/CssBaseline/CssBaseline.js

@@ -43,6 +43,10 @@ export const styles = (theme, enableColorScheme = false) => {
    'strong, b': {
      fontWeight: theme.typography.fontWeightBold,
    },
+    a: {
+      color:
+        theme.palette.mode === 'dark' ? theme.palette.primary[300] : theme.palette.primary[700],


Regardless of which color we choose, we should also add the support for theme.vars as well. I would recommend adding this value as a palette key and use that directly instead of hardcoding the values in the CssBaseline.

@mnajdova Can I ask you to double-check the second to last commit to see if I've done that correctly? 😬

brijeshb42 · 2023-11-27T13:52:56Z

packages/mui-joy/src/CssBaseline/CssBaseline.tsx

+            a: {
+              color:
+                theme.palette.mode === 'dark'
+                  ? theme.palette.primary[300]


Suggested change

? theme.palette.primary[300]

? (theme.vars || theme).palette.primary[300]

@brijeshb42 would an approach similar to what I pushed for the Material version work? It's following Marija's guidance and it definitely looks cleaner not having to call the dark mode ternary here 😬

brijeshb42 added component: CssBaseline The React component package: material-ui Specific to @mui/material labels Jul 24, 2023

brijeshb42 requested a review from danilo-leal July 24, 2023 09:48

siriwatknp reviewed Jul 25, 2023

View reviewed changes

[material] add colors with better constrast to <a>

e6253b0

in CssBaseline component for both light and dark modes

brijeshb42 force-pushed the material-css-baseline-a branch from 958e6ff to e6253b0 Compare July 25, 2023 06:34

brijeshb42 requested a review from siriwatknp July 25, 2023 06:35

oliviertassinari reviewed Jul 27, 2023

View reviewed changes

docs/data/material/components/css-baseline/css-baseline.md Outdated Show resolved Hide resolved

oliviertassinari changed the title ~~[material] add colors with better constrast to <a>~~ [CssBaseline] Add colors with better constrast to <a> Jul 27, 2023

danilo-leal reviewed Jul 31, 2023

View reviewed changes

docs/data/joy/components/css-baseline/css-baseline.md Outdated Show resolved Hide resolved

docs/data/material/components/css-baseline/css-baseline.md Outdated Show resolved Hide resolved

danilo-leal added package: joy-ui Specific to @mui/joy design This is about UI or UX design, please involve a designer labels Jul 31, 2023

Alim-El reviewed Aug 2, 2023

View reviewed changes

packages/mui-joy/src/CssBaseline/CssBaseline.tsx Show resolved Hide resolved

oliviertassinari mentioned this pull request Aug 16, 2023

[CssBaseline] Consider rework #38507

Open

mnajdova reviewed Nov 14, 2023

View reviewed changes

danilo-leal added 4 commits November 27, 2023 10:24

copywriting tweaks

2f37366

copy left-over from the previous commit

39d51d7

creating the link color theme key

68277f2

Merge branch 'master' into pr/38135

fa0a76a

brijeshb42 commented Nov 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CssBaseline] Add colors with better constrast to <a> #38135

[CssBaseline] Add colors with better constrast to <a> #38135

brijeshb42 commented Jul 24, 2023 •

edited

brijeshb42 commented Jul 24, 2023

mui-bot commented Jul 24, 2023 •

edited

zanivan commented Jul 24, 2023

brijeshb42 commented Jul 24, 2023

zanivan commented Jul 24, 2023

brijeshb42 commented Jul 24, 2023

zanivan commented Jul 24, 2023

siriwatknp Jul 25, 2023

brijeshb42 Jul 25, 2023

brijeshb42 Jul 25, 2023

oliviertassinari Jul 27, 2023 •

edited

oliviertassinari commented Jul 27, 2023 •

edited

brijeshb42 commented Jul 31, 2023

danilo-leal left a comment

oliviertassinari commented Jul 31, 2023

oliviertassinari commented Aug 1, 2023 •

edited

mnajdova Nov 14, 2023

danilo-leal Nov 27, 2023 •

edited

brijeshb42 Nov 27, 2023

danilo-leal Nov 27, 2023

	? theme.palette.primary[300]
	? (theme.vars \|\| theme).palette.primary[300]

[CssBaseline] Add colors with better constrast to <a> #38135

Are you sure you want to change the base?

[CssBaseline] Add colors with better constrast to <a> #38135

Conversation

brijeshb42 commented Jul 24, 2023 • edited

brijeshb42 commented Jul 24, 2023

mui-bot commented Jul 24, 2023 • edited

Netlify deploy preview

Bundle size report

zanivan commented Jul 24, 2023

brijeshb42 commented Jul 24, 2023

zanivan commented Jul 24, 2023

brijeshb42 commented Jul 24, 2023

zanivan commented Jul 24, 2023

siriwatknp Jul 25, 2023

Choose a reason for hiding this comment

brijeshb42 Jul 25, 2023

Choose a reason for hiding this comment

brijeshb42 Jul 25, 2023

Choose a reason for hiding this comment

oliviertassinari Jul 27, 2023 • edited

Choose a reason for hiding this comment

oliviertassinari commented Jul 27, 2023 • edited

brijeshb42 commented Jul 31, 2023

danilo-leal left a comment

Choose a reason for hiding this comment

oliviertassinari commented Jul 31, 2023

oliviertassinari commented Aug 1, 2023 • edited

mnajdova Nov 14, 2023

Choose a reason for hiding this comment

danilo-leal Nov 27, 2023 • edited

Choose a reason for hiding this comment

brijeshb42 Nov 27, 2023

Choose a reason for hiding this comment

danilo-leal Nov 27, 2023

Choose a reason for hiding this comment

brijeshb42 commented Jul 24, 2023 •

edited

mui-bot commented Jul 24, 2023 •

edited

oliviertassinari Jul 27, 2023 •

edited

oliviertassinari commented Jul 27, 2023 •

edited

oliviertassinari commented Aug 1, 2023 •

edited

danilo-leal Nov 27, 2023 •

edited