Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Define button color #5000

Merged
merged 1 commit into from
Oct 24, 2018
Merged

Define button color #5000

merged 1 commit into from
Oct 24, 2018

Conversation

guoyunhe
Copy link
Contributor

@guoyunhe guoyunhe commented Oct 5, 2018

In Linux desktop with dark theme, default button text color is white or light gray. If button background is set to a light color, button text are not visible. So button text color and background color must both be defined.

Before:

screenshot_20181005_120700

After:

screenshot_20181005_120727

In Linux desktop with dark theme, default button text color is white or light gray. If button background is set to a light color, button text are not visible. So button text color and background color must both be defined.
@jaredhirsch
Copy link
Member

@guoyunhe Thanks for the pull request. What linux distro and window manager are you using?

@guoyunhe
Copy link
Contributor Author

@6a68 openSUSE Tumbleweed + KDE 5.14.0 with Breeze Dark theme.

Copy link
Member

@jaredhirsch jaredhirsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request. I was curious about this issue, and a little digging turned up a long, sad history of Firefox applying system styles to websites, with odd results on Linux for certain dark themes--particularly themes that don't provide both light and dark options, but just default to dark. (https://bugzilla.mozilla.org/show_bug.cgi?id=70315 seems to be the original bug, in case anyone's interested in exploring.)

But, back to this PR. I'm hesitant to override the ButtonText system color with black, but I can't come up with a good reason not to. Windows high contrast mode forces the button label to white, so specifying black for the text color doesn't cause problems there. Moreover, we do already style the button pretty extensively. If this turns out to break the button labels for users of different linux themes, I'll probably have to revert the change. For now, I'll merge it and we'll see how it goes 👍

@jaredhirsch jaredhirsch merged commit 20d5321 into mozilla-services:master Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants