-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Ally-compliant color palette & functions #9319
Changes from 17 commits
9f80685
86acf95
98b8226
4bd3657
d3b0671
c720183
bf45095
2306a8a
e1ea3c6
30adb30
ccd29a8
3908e8d
44f4a2b
d2ad228
482407a
c630cba
a516f08
8f12ab5
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 |
---|---|---|
|
@@ -20,7 +20,7 @@ $accordion-title-font-size: rem-calc(12) !default; | |
|
||
/// Default text color for items in a Menu. | ||
/// @type Color | ||
$accordion-item-color: foreground($accordion-background, $primary-color) !default; | ||
$accordion-item-color: $primary-color !default; | ||
|
||
/// Default background color on hover for items in a Menu. | ||
/// @type Color | ||
|
@@ -40,16 +40,18 @@ $accordion-content-border: 1px solid $light-gray !default; | |
|
||
/// Default text color of tab content. | ||
/// @type Color | ||
$accordion-content-color: foreground($accordion-content-background, $body-font-color) !default; | ||
$accordion-content-color: $body-font-color !default; | ||
|
||
/// Default padding for tab content. | ||
/// @type Number | List | ||
$accordion-content-padding: 1rem !default; | ||
|
||
/// Adds styles for an accordion container. Apply this to the same element that gets `data-accordion`. | ||
@mixin accordion-container { | ||
@mixin accordion-container ( | ||
$background: $accordion-background | ||
) { | ||
margin-#{$global-left}: 0; | ||
background: $accordion-background; | ||
background: $background; | ||
list-style-type: none; | ||
} | ||
|
||
|
@@ -65,26 +67,32 @@ $accordion-content-padding: 1rem !default; | |
} | ||
|
||
/// Adds styles for the title of an accordion item. Apply this to the link within an accordion item. | ||
@mixin accordion-title { | ||
@mixin accordion-title ( | ||
$padding: $accordion-item-padding, | ||
$font-size: $accordion-title-font-size, | ||
$color: $accordion-item-color, | ||
$border: $accordion-content-border, | ||
$background-hover: $accordion-item-background-hover | ||
) { | ||
position: relative; | ||
display: block; | ||
padding: $accordion-item-padding; | ||
padding: $padding; | ||
|
||
border: $accordion-content-border; | ||
border: $border; | ||
border-bottom: 0; | ||
|
||
font-size: $accordion-title-font-size; | ||
font-size: $font-size; | ||
line-height: 1; | ||
color: $accordion-item-color; | ||
color: $color; | ||
|
||
:last-child:not(.is-active) > & { | ||
border-bottom: $accordion-content-border; | ||
border-bottom: $border; | ||
border-radius: 0 0 $global-radius $global-radius; | ||
} | ||
|
||
&:hover, | ||
&:focus { | ||
background-color: $accordion-item-background-hover; | ||
background-color: $background-hover; | ||
} | ||
|
||
@if $accordion-plusminus { | ||
|
@@ -103,18 +111,23 @@ $accordion-content-padding: 1rem !default; | |
} | ||
|
||
/// Adds styles for accordion content. Apply this to the content pane below an accordion item's title. | ||
@mixin accordion-content { | ||
@mixin accordion-content ( | ||
$padding: $accordion-content-padding, | ||
$border: $accordion-content-border, | ||
$background: $accordion-content-background, | ||
$color: $accordion-content-color | ||
) { | ||
display: none; | ||
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.
Please rebase, then run 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.
|
||
padding: $accordion-content-padding; | ||
padding: $padding; | ||
|
||
border: $accordion-content-border; | ||
border: $border; | ||
border-bottom: 0; | ||
background-color: $accordion-content-background; | ||
background-color: $background; | ||
|
||
color: $accordion-content-color; | ||
color: $color; | ||
|
||
:last-child > &:last-child { | ||
border-bottom: $accordion-content-border; | ||
border-bottom: $border; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,11 @@ $badge-background: $primary-color !default; | |
|
||
/// Default text color for badges. | ||
/// @type Color | ||
$badge-color: foreground($badge-background) !default; | ||
$badge-color: $white !default; | ||
|
||
/// Alternate text color for badges. | ||
/// @type Color | ||
$badge-color-alt: $black !default; | ||
|
||
/// Default padding inside badges. | ||
/// @type Number | ||
|
@@ -49,7 +53,7 @@ $badge-font-size: 0.6rem !default; | |
@if $name != primary { | ||
&.#{$name} { | ||
background: $color; | ||
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. We should rename 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'm not sure about this. The |
||
color: foreground($color); | ||
color: color-pick-contrast($color, ($badge-color, $badge-color-alt)); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ $orbit-control-zindex: 10 !default; | |
padding: $orbit-caption-padding; | ||
|
||
background-color: $orbit-caption-background; | ||
color: foreground($orbit-caption-background); | ||
color: color-pick-contrast($orbit-caption-background); | ||
background-color: $orbit-caption-background; | ||
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. "background-color: $orbit-caption-background;" is duplicated line 98 and 100 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. Ack! Must've missed that when rebasing and fixing a merge conflict. I'll clean this up… |
||
} | ||
|
||
/// Adds base styles for the next/previous buttons in an Orbit slider. These styles are shared between the `.orbit-next` and `.orbit-previous` classes in the default CSS. | ||
|
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.
If you have a reference for these colors, can you add the link in comment ?
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'm not sure I understand what you're asking for. Do you mean adding something like this as an inline comment for each of the colors?
// http://webaim.org/resources/contrastchecker/?fcolor=ffffff&bcolor=cc4b37
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 mean : How did you find that we should use these particular colors, and not, for example,
#cc4b38
?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 used Photoshop to incrementally adjust the original color's luminosity (keeping the same hue and saturation, changing only the brightness), checking each incremental change in the WebAIM Contrast Checker, until the change allowed the new color to meet AA requirements.
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.
Ok. I think it could be useful to add the link to test the contrast, with an explanation.