Ally-compliant color palette & functions #9319

andycochran · 2016-11-01T20:34:58Z

This PR, which effects every component that used the foreground() function, addresses #7387. I've adjusted the $foundation-palette to meet AA contrast requirements. The foreground() function has been replaced with a new pick-best-color() function that uses luminosity instead of lightness (which fails as #7387 explains).

andycochran · 2016-11-02T20:09:43Z

While replacing foreground() with a function that uses luminosity, I noticed that Tabs is using foreground() for the content font color. There's really no reason for this, as the content background color is explicitly set. Now $tab-content-color is just simply set to $body-font-color.

I also noticed that Tabs' mixins didn't allow for passing any vars when creating your own tabs. Fixed this too.

Finally, I added settings for tab item font colors. You could previously set the tab items' background, but not their color.

andycochran · 2016-11-02T21:33:35Z

Just like Tabs, it's the same case for Accordion & Pagination (foreground function sets the text color but the background color is explicitly set, and mixins don't pass vars). Fixing those too…

ncoden · 2016-11-02T23:35:56Z

👍
Wouldn't color-contrast() / color-*() / pick-contrast-color() (but I prefer a function with a namespace) be better than pick-best-color() ?

andycochran · 2016-11-02T23:55:39Z

I can change that to whatever everyone agrees is most semantic. That's just what the function is called in the gist from which I ported the luminosity functions. I've updated the function (and its description which was old/wrong) based on your feedback. Thanks, @ncoden!

kball · 2016-11-08T23:52:11Z

Travis failure appears to be real - @andycochran can you take a look?

ncoden · 2016-11-09T00:18:19Z

I don't know why sass-true crash now. But we can try with node v4.0.

kball · 2016-11-09T00:38:47Z

v4.2.2 throws the same error:

Error: Undefined operation: "0.2126 times pow(0.99628, 2.4)".
    at Object.module.exports.renderSync (/Users/kball/git/foundation-sites/node_modules/n
ode-sass/lib/index.js:440:16)
    at Object.module.exports.runSass (/Users/kball/git/foundation-sites/node_modules/sass
-true/lib/main.js:34:18)
    at Object.<anonymous> (/Users/kball/git/foundation-sites/test/sass/test_sass.js:14:10
)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)

kball · 2016-11-09T23:11:58Z

Found the issue. The scss tests import files individually... this PR introduces a dependency for color on math. You can fix the failure by adding an @import 'math' into _color.scss

kball · 2016-11-09T23:15:30Z

I can fix the above issue as a part of the merge if you like. Other than that, is this ready to go @andycochran @ncoden?

andycochran · 2016-11-10T01:15:58Z

@kball fixed. I think this is good to go so long as Zurb is cool with the new ally-compliant color palette. It's not as vibrant. I also wouldn't mind a "LGTM" from @ncoden or @brettsmason.

andycochran · 2016-11-10T01:18:10Z

Hmm… looks like Travis is still upset.

kball · 2016-11-10T01:20:54Z

Bah, kicking travis, looks like that spurious phantom download error.

@zurbrandon @tdhartwick @gakimball any of you have thoughts on this new default palette?

ncoden · 2016-11-10T01:56:53Z

scss/components/_badge.scss

@@ -47,7 +51,7 @@ $badge-font-size: 0.6rem !default;
      @if $name != primary {
        &.#{$name} {
          background: $color;


We should rename $color by $background here

I'm not sure about this. The $foundation-palette is a list of colors, not a list of backgrounds. We're using a $color from the palette to set a background. Seems logical as-is to me.

ncoden · 2016-11-10T02:02:30Z

scss/components/_accordion.scss

-  font-size: $accordion-title-font-size;
-  color: $accordion-item-color;
+  font-size: $font-size;
+  color: $color;


Why does some components have an alternative color, and others doesn't ?
It could be useful to use a common color format like a ($color-light, $color-dark) list (for background too), and a mixin like component-skin($background-list: (), $color-list: (), $pick-best-contrast: true) to use it in components.

I avoided -light and -dark because the default button and label do not use the $pick-best-contrast function. You might make an app with a dark background and a bright default button with dark text. If necessary, the buttons created from the palette use the alt color, which might be light.

Oh, and I'm only using alt colors for component that create classes with an @each loop for the $foundation-palette. Components that do not use a $foundation-palette loop do not need an alt color.

ncoden · 2016-11-10T02:04:45Z

scss/components/_accordion.scss

  display: block;
-  padding: $accordion-item-padding;
+  padding: $padding;
  line-height: 1;


font-size before line-height

ncoden · 2016-11-10T02:06:20Z

scss/components/_accordion.scss

+  $background: $accordion-content-background,
+  $color: $accordion-content-color
+) {
+  padding: $padding;
  display: none;


display before padding

Please rebase, then run gulp sass:lint ;)

gulp lint:sass 😉

ncoden · 2016-11-10T02:07:09Z

scss/_global.scss

  success: #3adb76,
  warning: #ffae00,
-  alert: #ec5840,
+  alert: #cc4b37,


If you have a reference for these colors, can you add the link in comment ?

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

I mean : How did you find that we should use these particular colors, and not, for example, #cc4b38 ?

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.

Ok. I think it could be useful to add the link to test the contrast, with an explanation.

andycochran · 2016-11-10T04:02:30Z

@ncoden Thanks for the feedback. I'll work on this tomorrow.

andycochran · 2016-11-10T20:12:13Z

FYI, here's the diff in the colors:

ncoden · 2016-11-11T03:16:16Z

Can't we keep background colors for a white text ?

andycochran · 2016-11-11T20:31:32Z

@ncoden In order to keep white text for the Success and Warning buttons, the colors have to darken quite a lot. The gold ends up being an ugly brown.

In order to change the palette as little as possible—and because I like the existing green/gold—I decided to leave the Success and Warning colors as they are. They meet AA requirements with the alt text color.

ncoden · 2016-11-12T08:57:18Z

@andycochran I understand. But the different text colors disturb me. And I don't find the disabled text very visible.

Maybe we should change the hue, saturation and luminosity to have beautiful and ally-compliant background colors without different text colors.

andycochran · 2016-11-12T16:02:40Z

@ncoden I'd love it if we could come up with that palette. But AA contrast requirements quite limiting. I think we can only get two out of these three: 1) beautiful 2) ally-compliant 3) white text. Please feel free to propose new colors.

I imagine most designers who don't care about AA requirements will simply set [component]-color and [component]-color-alt to the same value—e.g. white. 😢

Maybe the alt text color is visually awkward because all the buttons are shown in a row here? In practice, buttons won't often be arranged this way.

ncoden · 2016-11-12T16:21:01Z

@andersonaguiar
I'll search for better colors that meet these three critera. Otherwise, I would prefer beautiful and white text. I care about a11y, but not if it break the design for everyone.

Also, In practice, I often see a primary button next to a warning button. :/

andycochran · 2016-11-12T22:58:26Z

☹️ Caring more about aesthetics than a11y is directly opposed to the spirit of #7387.

@kball / @rafibomb, can Zurb and its designers can weigh in on this?

ncoden · 2016-11-12T23:49:37Z

I checked with WebAIM, and didn't find a a11y-compliant (AA) color for a white text that is not ugly 😞. Also, disabled button have a .25 opacity, so the background color is not relevant at all for a11y tests.

But could we, instead of changing the default colors, provide a set of tools to help websites to provide an alternative A11y High Contrast color palette dynamically (like DuckDuckGo).

Also, Firefox allows the user to overriding the page colors (but not Chrome).

andycochran · 2016-11-13T01:20:40Z

Don't worry about contrast for .disabled.

But even tough the guidelines do not require a contrast ratio for disabled UI components, I think the current style for the disabled buttons (opacity: 0.25) is a bit too light/has too low contrast ratio. Maybe the number could be tweaked at least a little bit.
– @Owlbertz's comment in #7387

Again, I think this comes down to a judgement Zurb should make. Should Foundation Sites actually be "fully-accessible" as advertised, and must that include meeting AA contrast requirements out-of-the-box?

Owlbertz · 2016-11-13T09:41:47Z

We are focusing so much on accessibility with the framework that imo we should make it be compliant out of the box. Of course this is a thing that the ZURBians have to decide, but as we are promoting to be "fully-accessible" as @andycochran said this should be the goal. Maybe we have to go even further and add some contrast checking to the SASS and/or the "custom download" page.

The biggest problem with a11y will always be that people are not aware of it or just don't care and thus we should give them as much help as we can to create compliant websites so they'll actually do.

mandimey · 2016-11-14T18:33:39Z

The updated compliant palette @andycochran suggested is a super solid compromise. @shannon-zurb and I tinkered with the palette this morning and arrived at a similar conclusion - white text on green and yellows is hard to make look beautiful and accessible.

Question came up in the office here - should we make the compliant palette a variable that can be disabled (or enabled)? We fear that defaulting the framework's color palette to be compliant maybe regressive. Sites may have intentionally used the current out-of-the-box colors because it aligns with their brand. @zurbrandon @tdhartwick

andycochran · 2016-11-14T22:08:16Z

@MandiSaeteun I don't think an extra variable just to turn on a11y colors is necessary—especially if it's on by default. Just as easily as turning it off, you could set your alt-colors to white. If this was a patch, I might agree that we shouldn't break folks' button colors. But it's a minor version with other changes that require reviewing your _settings.scss. Isn't advocating for AA compliance is more important than not bothering people to review the new version's settings?

andycochran · 2016-11-17T17:56:11Z

Looks like there are some new conflicts. I'll get these fixed…

@kball / @MandiSaeteun, any further thoughts from Zurb on out-of-the-box compliance?

… pass vars

IllusionPerdu · 2016-11-19T09:46:50Z

I think the callout have similar problem, in the setting they are a $callout-font-color-alt but never used and it sould be great if the background is dark the alt color is used.

IllusionPerdu · 2016-11-25T09:08:44Z

scss/components/_orbit.scss

@@ -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;


"background-color: $orbit-caption-background;" is duplicated line 98 and 100

Ack! Must've missed that when rebasing and fixing a merge conflict. I'll clean this up…

kball · 2016-11-28T18:27:12Z

@andycochran Our consensus has been that out of the box compliance is very important, and worth sacrificing a little bit of backwards compatibility. Lets write a migration note that addresses this and lets people know how they can revert to the previous palette. Other than this, is this ready to merge?

andycochran · 2016-11-28T18:44:51Z

@kball AFAIK, it's ready to merge. Didn't wanna add that label to my own PR tho.

kball · 2016-11-28T19:04:18Z

Merged! @andycochran can you write a quick migration note (comment in this PR would be fine) highlighting what the change is and how someone would get back to the previous palette if they wanted to?

andycochran · 2016-11-28T22:27:23Z

Migration notes

New functions

The foreground() function has been replaced with a new color-pick-contrast() function that uses luminosity instead of lightness.
The color-pick-contrast() function requires new _math.scss utilities for 1) finding the greatest common divisor of two integers and 2) handling decimal exponents by trying to convert them into a fraction and then use a nth-root-algorithm for parts of the calculation.

Revised settings variables

The $foundation-palette variables have been adjusted to meet WCAG 2.0 level AA contrast requirements. For backwards compatibility (ignoring AA requirements), revert primary to #2199e8, secondary to #777, and alert to #ec5840.
All instances of the foreground() function have removed from _settings.scss in favor of setting explicit colors and alternate colors.
The $tab-content-color and $pagination-item-color-current variables are now set explicitly. There's no need for a function to determine these color, as their associated background colors are also set explicitly. Previously, the foreground() function for Pagination only allowed for black or white text.

New settings variables

The Badges and Labels components now have alternate colors: $badge-color-alt and $label-color-alt. These are used the same way as $button-color-alt. The $badge-color and $label-color now default to white (like $button-color). Previously, the badge and label colors defaulted to a function which made assumptions about the lightness of the background colors. Buttons, Badges, and Labels components now all work the same way. The default can be any foreground/background color combination, and for each background color defined in $foundation-palette, the foreground is automatically set to the color or alternate color (whichever has more contrast). For backwards compatibility (ignoring AA requirements so all button colors have white text), set the alt colors to white.
The Tabs component now allows you to set the color of tab titles with $tab-color and $tab-active-color. Previously, tab titles were hardcoded to the primary color, which was not visible on some tab background colors.

Mixins

Accordion, Pagination, Tabs now allow for passing vars when creating your own components.

kball · 2016-12-02T00:01:58Z

@andycochran I think it would be valuable to add a note to the docs for buttons, badges, and labels that explains why they have mixed text colors out of the box, and how to change it to all white (with a warning that it will reduce your contrast below AA contrast requirements)

andycochran · 2016-12-02T00:26:31Z

@kball Will do…

andycochran changed the title ~~Allypalette~~ Ally-compliant color palette & functions Nov 1, 2016

andycochran mentioned this pull request Nov 2, 2016

Meet AA contrast requirements #7387

Closed

6 tasks

ncoden reviewed Nov 10, 2016

View reviewed changes

andycochran force-pushed the allypalette branch from a9cbf60 to 0812226 Compare November 10, 2016 19:57

Andy Cochran and others added 10 commits November 17, 2016 13:17

add badge-color-alt var for pick-best-color()

2306a8a

add label-color-alt var for pick-best-color()

e1ea3c6

ditch function for accordion content color, allow accordion mixins to…

30adb30

… pass vars

ditch function for pagination item color

ccd29a8

allow pagination mixins to pass vars

3908e8d

pick-best-color() to color-pick-contrast(), update its comment

44f4a2b

import math function in color scss

d2ad228

replace all instances of foreground() with pick-best-color()

482407a

bring back button-color-alt

c630cba

pick-best-color() to color-pick-contrast(), update its comment

a516f08

andycochran force-pushed the allypalette branch from 0812226 to a516f08 Compare November 17, 2016 18:34

IllusionPerdu reviewed Nov 25, 2016

View reviewed changes

nix duplicate line

8f12ab5

kball added the 💥 migration needed label Nov 28, 2016

kball merged commit 86ebf14 into foundation:develop Nov 28, 2016

andycochran mentioned this pull request Nov 28, 2016

rename tab-background-active-color to tab-active-color #9411

Merged

kball added this to the 6.3 milestone Nov 28, 2016

andycochran mentioned this pull request Dec 2, 2016

update button,badge,label docs for new a11y palette #9433

Merged

wrburgess mentioned this pull request Dec 15, 2016

foreground() causes error in 6.3.0 gem upgrade foundation/foundation-rails#214

Closed

This was referenced Dec 24, 2016

Customerizer wrong default color #9555

Closed

Color of the .button.success #9564

Closed

kball mentioned this pull request Jul 5, 2017

Accessibility conformance commitment level #10348

Closed

renovate bot mentioned this pull request Aug 7, 2021

chore(deps): update dependency foundation to v6 uPortal-Project/resource-server#184

Closed

1 task

Ally-compliant color palette & functions #9319

Ally-compliant color palette & functions #9319

Conversation

andycochran commented Nov 1, 2016 • edited by ncoden

andycochran commented Nov 2, 2016

andycochran commented Nov 2, 2016

ncoden commented Nov 2, 2016

andycochran commented Nov 2, 2016

kball commented Nov 8, 2016

ncoden commented Nov 9, 2016

kball commented Nov 9, 2016

kball commented Nov 9, 2016

kball commented Nov 9, 2016

andycochran commented Nov 10, 2016

andycochran commented Nov 10, 2016

kball commented Nov 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncoden Nov 10, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andycochran commented Nov 10, 2016

andycochran commented Nov 10, 2016

ncoden commented Nov 11, 2016

andycochran commented Nov 11, 2016

ncoden commented Nov 12, 2016 • edited

andycochran commented Nov 12, 2016

ncoden commented Nov 12, 2016

andycochran commented Nov 12, 2016

ncoden commented Nov 12, 2016 • edited

andycochran commented Nov 13, 2016

Owlbertz commented Nov 13, 2016

mandimey commented Nov 14, 2016

andycochran commented Nov 14, 2016

andycochran commented Nov 17, 2016

IllusionPerdu commented Nov 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kball commented Nov 28, 2016

andycochran commented Nov 28, 2016

kball commented Nov 28, 2016

andycochran commented Nov 28, 2016 • edited

Migration notes

New functions

Revised settings variables

New settings variables

Mixins

kball commented Dec 2, 2016

andycochran commented Dec 2, 2016

andycochran commented Nov 1, 2016 •

edited by ncoden

ncoden Nov 10, 2016 •

edited

ncoden commented Nov 12, 2016 •

edited

ncoden commented Nov 12, 2016 •

edited

andycochran commented Nov 28, 2016 •

edited