-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
update node-green to pass WCAG AA #536
Changes from 2 commits
3288ff6
b2bbd2f
0479b57
176ce7a
85472cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ $body-max-width = 980px | |
|
||
$white = #fff | ||
|
||
$node-green = #80bd01 | ||
$node-green = #43853d | ||
$bright-green = #028a00 | ||
$light-green = #f1fbda | ||
$hover-green = #d9ebb3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these two be updated with the new color guide? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll take a look at where/how they are being used |
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,7 @@ | |
margin 10px 4px | ||
padding 0.2em 0.6em | ||
|
||
background-color $node-green | ||
background-color $bright-green | ||
color $white !important | ||
border-radius 2px | ||
font-size 30px | ||
|
@@ -51,7 +51,7 @@ | |
transition .2s background-color ease-in-out | ||
|
||
&:hover | ||
background-color saturation($node-green, 80%) | ||
background-color darken($bright-green, 20%) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then this could use |
||
|
||
small | ||
display block | ||
|
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.
Why not
#44883e
(http://rgb.to/pantone/7741-c)? I'd say it's good enough contrast.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.
I think that was the original spec color (correct me if I'm wrong) and it turned out to just barely not pass WCAG, which was actually the impetus for this PR originally. It is my hope in the future we can include WCAG validation as part of the site build process, so even the minor difference would cause a failure. Just taking care of the most important contrast differences for now, baby steps.
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.
Well, It's barely noticeable, so I don't have much of an opinion, but I'd lean on more the side of using the original color than to make it match the guideline.