Raised button on dark theme uses primary-on-dark text even though the background is primary, not dark

[unascribed]

What MDC-Web Version are you using?

0.13.0

What browser(s) is this bug affecting?

Chromium 58.0.3029.81

Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Firefox Developer Edition 54.0b12

Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0

What OS are you using?

Arch Linux x86_64 (last updated May 3rd)

What are the steps to reproduce the bug?

1. Create a raised button
2. Add mdc-theme--dark to a parent element
3. Set the primary color to something bright, like Yellow 500
4. Observe the fact the button is completely illegible

https://codepen.io/anon/pen/EXyBGe

What is the expected behavior?

The button text color should change to black, as if it was using --mdc-theme-text-primary-on-primary

What is the actual behavior?

The button text color stays white, as it's using --mdc-theme-text-primary-on-dark

[touficbatache]

That seems like a bug!

[makotosalon]

Please unsubscribe. 2017/06/14 5:06、William Thompson <notifications@github.com> のメッセージ: What MDC-Web Version are you using? 0.13.0 What browser(s) is this bug affecting? Chromium 58.0.3029.81 Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36 Firefox Developer Edition 54.0b12 Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0 What OS are you using? Arch Linux x86_64 (last updated May 3rd) What are the steps to reproduce the bug? Create a raised button Add mdc-theme--dark to a parent element Set the primary color to something bright, like Yellow 500 Observe the fact the button is completely illegible https://codepen.io/anon/pen/EXyBGe What is the expected behavior? The button text color should change to black, as if it was using --mdc-theme-text-primary-on-primary What is the actual behavior? The button text color stays white, as it's using --mdc-theme-text-primary-on-dark ― You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[touficbatache]

However, you can do this manually by adding the mdc-theme--text-primary-on-light class to the button.

[traviskaufman]

Hey @unascribed,

Thanks for reporting!

I actually think this looks like there's some incorrect styles being applied when using a raised button in dark mode. Looking at our code:

// .mdc-button--raised

  @include mdc-theme-dark(".mdc-button") {
      @include mdc-theme-prop(background-color, primary);
      // ...
  }

I'm not sure it's correct to set the background-color to the theme's primary color for raised buttons. I'd accept a PR removing this, instead just having the button use a default BG color (perhaps inherit?)

@material-components/mdc-web thoughts?

[unascribed]

Using the primary color as the background for default buttons on the dark theme seems to be correct according to the guidelines:

And it's a bit hard to tell raised uncolored dark buttons from the background, since the shadow is black. In my case, I'm building an adaptive UI with user-set colors, including a dark/light switch, so having the raised default buttons look like flat ones in the dark theme is undesirable.

At the same time, though, that's only my use case, and since MDC-Web gives full control over the button background color with classes, it's likely a moot point. In my case specifically I can easily re-add this behavior with a few lines of CSS.

Will try making a PR, but first I'll have to make some sense of the mixin soup. 😛

[unascribed]

Actually, I think I like the uncolored buttons:

Of course, that's your decision, not mine.

[lynnmercier]

Sorry for the confusion here. I've taken a look at this issue, and the related pull request...and I've talked to our MD team.

The spec does state that raised buttons on dark theme should use primary color. It says "Normal color: 500", which I confirmed is the primary color. You could also use the accent color, but MDC Web's code will default to the primary color.

The real issue is using #FFEA00 (bright yellow) as a "primary color" doesn't work with white text. @makotosalon demonstrated this with his codepen

Using bright yellow as a primary color means --mdc-theme-text-primary-on-primary should be set to a darker color. You can use this color tool (slide three shows the darker text on a yellow button)
Here is a new codepen demonstrating the changes to this --mdc-theme-text-primary-on-primary.

I've created a new PR with a fix to include mdc-theme-text-primary-on-primary on raised buttons on dark theme. Hopefully that removes @Garbee concerns with the first pull request.

I would also like to close out the older pull request.