-
Notifications
You must be signed in to change notification settings - Fork 26
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
Replaced mid_grey with dark_grey #1909
Conversation
Text using the $color_mid_grey variable wasn't passing the contrast test(3.11), even against #fff. By using $color_dark_grey instead the contrast test is passing with 5.32. Still gets a muted effect.
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.
Minor change needed for the locale list - see video attached.
@@ -1217,7 +1217,7 @@ p.locale-list-trigger { | |||
border-bottom: none; | |||
background-color: rgba(0,0,0,0.1); | |||
@include respond-min($main_menu-mobile_menu_cutoff) { | |||
background-color: darken($color_mid_grey, 10%); | |||
background-color: darken($color_dark_grey, 10%); |
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.
This breaks the hover effect:
locale-hover.mov
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.
@gbp Thanks for the video. I just added a fixup
that should fix the issue. I kept the mid-grey
but increased the darkening factor
to 15%
now the contrast ratio is at 5.33
. However I did it for the default state and not the hover one. The default one was failing the contrast test.
@@ -977,7 +977,7 @@ div.correspondence { | |||
|
|||
font-weight: 600; | |||
strong { | |||
color: $color_mid_grey; | |||
color: $color_dark_grey; |
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.
@@ -1888,7 +1888,7 @@ dd { | |||
} | |||
|
|||
.no-entries { | |||
color: $color_mid_grey; | |||
color: $color_dark_grey; |
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.
Relevant issue(s)
While working on a blog post I noticed the contrast test of anything using
color_mid_grey
against a light background was failing the contrast test(3.11). No issue has been formally created.What does this do?
Text using the
$color_mid_grey
variable wasn't passing the contrast test(3.11), even against#fff
. By using$color_dark_grey
the contrast test is passing with 5.32. Still gets a muted effect.Screenshots
Notes to reviewer