-
Notifications
You must be signed in to change notification settings - Fork 656
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
fix disabled button color #1184
Conversation
body[class="page-responsive"] .btn-primary .octicon { | ||
color: hsla(0, 0%, 100%, .8); | ||
color: hsla(0, 0%, 100%, .3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here at this value the octicon is going to break because $color
thinks this is OK. SO we fix one and break others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we see in order to fix the disabled buttons text color we will break other things, and @silvewind keeps adding $color everywhere :(
.btn-primary .octicon { | ||
color: hsla(0, 0%, 100%, .8); | ||
color: hsla(0, 0%, 100%, .3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
.section-codespaces-develop-night .codespaces-develop-text { | ||
color: hsla(0, 0%, 100%, .8) !important; | ||
color: hsla(0, 0%, 100%, .3) !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
.flash-header { | ||
color: hsla(0, 0%, 100%, .8); | ||
color: hsla(0, 0%, 100%, .3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
html[class^="ghh"] .ghh button.ghh-aux { | ||
color: hsla(0, 0%, 100%, .8); | ||
color: hsla(0, 0%, 100%, .3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
@@ -34687,9 +34687,9 @@ | |||
.cpt-notification { | |||
color: #fff; | |||
} | |||
/* githubstatus.com: "rgba(255,255,255,0.8)" */ | |||
/* githubstatus.com: "color: hsla(0,0%,100%,.8)" */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
.layout-content.status.status-index .page-status .last-updated-stamp { | ||
color: hsla(0, 0%, 100%, .8); | ||
color: hsla(0, 0%, 100%, .3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will break.
The main issue here is that a inversion approach does not cleanly work because GH uses white text and we do too (ideally we should have black text but that won't work either). I see no good solution except to manually override the disabled button selectors unfortunately. |
Ideally we wouldnt fix what isnt broken, buttons use to work and already manual styles, it seems like were going backwards here. |
This is how a disabled button looks,
Looks anything BUT.
re: #1154 issue 2
The reason why I PR this is for @silverwind to see how using
$color
for buttons is less than ideal idea because whats good in one value for some property in a rule elsewhere isnt necessarily good for another rule with a different property, in this case this PR is good for disabled button text color but not good for other rules.Read the diff and see the wack-a-mole game
I cant commit this in good conscience and manual fixes at this level is not something I want to revisit.