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

mdc-button in Edge browser is colorless #1102

Closed
ghost opened this issue Aug 9, 2017 · 7 comments
Closed

mdc-button in Edge browser is colorless #1102

ghost opened this issue Aug 9, 2017 · 7 comments
Assignees

Comments

@ghost
Copy link

ghost commented Aug 9, 2017

Renders properly in Chrome and FF
untitled

@touficbatache
Copy link
Contributor

Hey, thanks for filing an issue. But please consider using the provided template to submit bugs / issues as it makes it more clear what the issue is, which browser is it affecting, which OS you're using, etc... so it can be tested and fixed as soon as possible.

@acdvorak acdvorak added the bug label Aug 11, 2017
@acdvorak acdvorak assigned acdvorak and yeelan0319 and unassigned acdvorak Aug 11, 2017
@acdvorak
Copy link
Contributor

Definitely looks like a bug. I was able to repro it on our button demo page in Edge 15.

It's not just dark mode, either: all --raised buttons seem to lack any background color at all in Edge.

As @touficbatache suggested, in the future, please try to use the provided issue template and attach a CodePen (or in this case, a link to the public demo page). It will help us get to the bottom of the issue faster.

Thanks!

@RobJacobs
Copy link
Contributor

@acdvorak I think this has to do with when the global css variables are set/imported. On the demo page, I used the dev tools to look at the 'Buttons - RAISED WITH PRIMARY' element - the computed tab revealed:

image

For the background-color property, notice the .mdc-button--primary.mdc-button--raised: var(---mdc-theme-primary, #212121) from the assests/demo-styles.css.js is being overruled by the .mdc-button: transparent rule from the assets/material-components-web.css.js.

Odly enough, simply unchecking the top level background-color checkbox will cause the rule to evaluate correctly. Rechecking will then apply the rules:

image

Same applies to the button color rule.

@yeelan0319
Copy link
Contributor

yeelan0319 commented Aug 14, 2017

@RobJacobs , thanks for the help! I found the same issue as you do where the more specific rule get override by generic rule, but this only happen to the latest version of Edge (15).

@acdvorak @Loggan08: This seems to be a Edge 15 only bug because its wrong implementation of pseudo element style. I will try to lock down this further.

@RobJacobs
Copy link
Contributor

@yeelan0319 @acdvorak I was hoping PR #1113 would fix the problem, but no luck.

@kfranqueiro
Copy link
Contributor

I've done some more investigation on this, and it seems like probably another issue with Edge, CSS variables, and pseudo-elements - but in this case, the CSS variable that trips it is applied to the root element, not the pseudo-elements.

I've been looking at FAB which has a similar issue (it uses secondary for its background), and if I switch its background-color to use a color literal rather than a CSS variable, the color renders correctly. (Alternatively, if I remove the ripple styles that apply background-colors to the pseudo-elements, the issue also goes away; interestingly, those values don't even reference CSS variables in the particular case I'm looking at.)

I suspect this may boil down to us needing to exclude at least some usages of CSS variables from Edge until they sort out their own browser. This unfortunately still happens even on Edge 16 in the modern.ie Win10 preview VM. My biggest concern in working around this is how much CSS I'll end up needing to add, which would affect load times in all browsers.

@kfranqueiro
Copy link
Contributor

This was fixed via #1364 which was merged manually.

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

No branches or pull requests

5 participants