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

[Accordion] Summary low contrast on focus in dark mode #20692

Closed
2 tasks done
petermikitsh opened this issue Apr 22, 2020 · 4 comments
Closed
2 tasks done

[Accordion] Summary low contrast on focus in dark mode #20692

petermikitsh opened this issue Apr 22, 2020 · 4 comments
Labels
accessibility a11y bug 🐛 Something doesn't work component: accordion This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@petermikitsh
Copy link
Contributor

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Screen Shot 2020-04-22 at 12 47 20 AM

Expected Behavior 🤔

The summary text should be more legible.

Steps to Reproduce 🕹

Steps:

  1. Go to https://material-ui.com/components/expansion-panels/#expansion-panel
  2. Enable dark mode
  3. Use tab key to navigate to an expansion panel summary element

Context 🔦

The context is accessibility.

Inputting the values into https://webaim.org/resources/contrastchecker/:

Screen Shot 2020-04-22 at 12 53 03 AM

@marcosvega91
Copy link
Contributor

marcosvega91 commented Apr 22, 2020

Maybe for darken theme is better to use 600: '#757575'; instead of 300: '#e0e0e0';

@oliviertassinari oliviertassinari added the component: accordion This is the name of the generic UI component, not the React module! label Apr 22, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 22, 2020

Wow, it has been broken like this for 2 years 🙃. How is this even possible. Do people use the accordion or dark mode at all 😱?

What about this diff?

diff --git a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
index b61281403..c476e7d30 100644
--- a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
+++ b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
@@ -25,10 +25,10 @@ export const styles = (theme) => {
         minHeight: 64,
       },
       '&$focused': {
-        backgroundColor: theme.palette.grey[300],
+        backgroundColor: theme.palette.action.focus,
       },
       '&$disabled': {
-        opacity: 0.38,
+        opacity: theme.palette.action.disabledOpacity,
       },
     },
     /* Pseudo-class applied to the root element, children wrapper element and `IconButton` component if `expanded={true}`. */

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. labels Apr 22, 2020
@marcosvega91
Copy link
Contributor

I have tried and can be good

@marcosvega91
Copy link
Contributor

Am i mistake or I saw a PR for this?

@oliviertassinari oliviertassinari changed the title [ExpansionPanelSummary] low contrast on focus in dark mode [AccordionSummary] low contrast on focus in dark mode Jul 18, 2020
@oliviertassinari oliviertassinari changed the title [AccordionSummary] low contrast on focus in dark mode [Accordion] Summary low contrast on focus in dark mode Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y bug 🐛 Something doesn't work component: accordion This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

3 participants