[Amsterdam] Saturating colors and providing contrast mixin for text.

[daveyholler]

Design (@daveyholler) Summary

• Saturating primary, secondary, and accent colors.
• Adding text specific variables for primary, secondary, accent, warning and updating components to use them in their own scss variables/loop functions
• Adding a second variable to provide contrasty versions for text

Semi-break:

Three of the core colors in the Amsterdam theme have been brightened (saturated) to bring a little more vibrancy into our applications. This change resulted in lower-than-acceptable contrast ratios for certain items like text and icons. To account for this change and to continue supporting a minimum contrast ratio of 4.5:1, we've added a few text-specific color variables. These variables are now used by default in Amsterdam and the current EUI theme in EuiText, EuiLink, EuiStat, and EuiExpression.

• euiColorPrimaryText
• euiColorSecondaryText
• euiColorAccentText
• euiColorWarningText

Consumers

Any usages of EUI color variables to style text-only elements outside of an EUI component (such as EuiText) will need to be updated use one of the new text-specific colors above upon switch to the new Amsterdam theme.

---

Dev (@cchaos) Summary

Consumers

The good news is that there are only a handful of direct usages of $euiColorSuccess/$euiColorSecondary in Kibana’s SASS. Though I’m not sure how many usages of the JSON token there are in JS. This means, at least in Kibana, updating these usages to ensure contrast shouldn’t be too hard.

Tweaks to downstream components

As the process goes down further into individual components there are probably a few that will want to be tweaked like EuiNotificationBadge.

Yet another color function

As I was checking for any other contrast issues with the new colors, one place that I noticed had changed were the app icons. The secondary color was using the unmodified version of $euiColorSecondary which has too low of contrast. However, the WCAG spec says that graphics don’t need to be a full 4.5:1 ratio but simply at least 3:1.

I created a new function called makeGraphicColorContrast() That is just a reuse of makeHighContrastColor but with a ratio of 3. This allows the secondary color in icon usages to be a bit brighter.

However, this does cause some issues with inconsistencies in color when text and icons are used together, like in EuiStat. Is this acceptable?

Icons are lighter than the text

Tweaks to makeHighContrastColor()

This function always forced the consumer to pass a background color and didn’t allow for adjusting the ratio (if needed). I just added some basic defaults to these parameters; $euiPageBackgroundColor for the background and 4.5 for the ratio.

Bug found: It has been noticed, however, that the math functions we have in the code are incorrect some of the times. #2916 was created to track this.

EUI default theme

None of these changes should effect the current aesthetics of the default theme. All of the base colors (except warning) were already of the proper contrast ratio therefore these extra functions don't change those values. The only change you might see is in icons that get passed the warning color will actually now get the proper contrast.

Checklist

• Check against all themes for compatibility in both light and dark modes
  - [ ] Checked in mobile
• Checked in IE11 and Firefox
  - [ ] Props have proper autodocs
• Added documentation examples
  - [ ] Added or updated jest tests
  - [ ] Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[snide]

Quick question

[cchaos]

So one of the quickest ways you can test your color changes against the current theme and any affects they will have on color contrast compliance is by checking the Colors Guidelines page.

Here's a screenshot of EUI vs Amsterdam showing a minimum ratio of 3. We can already see that we've lost contrast compliance in two main colors: secondary and accent.

Since the current EUI theme assumes compliance of these colors (on $euiColorEmptyShade), we don't do any alterations of these main colors when using them in other components like EuiTextColor, EuiLink, or EuiExpression.

Therefore, we can't quite merge this as-is. The consideration is, should the main variable $euiColorSecondary continue to serve as text on white, a11y acceptable color (not breaking any consumers intentions) or should each component that uses these colors create a text-appropriate variant?

[snide]

or should each component that uses these colors create a text-appropriate variant

Old designer opinion. The more tokens you have the more you need to maintain and the harder it is for people to jump into your code and understand how it works. Remember that all this then cascades into the dark/light themes the reverse contrast (white on $color buttons). It gets complicated fast. I'd suggest trying to keep your base palette above AA against white without fudging.

[daveyholler]

Maybe I'm not following something quite right...

@cchaos regarding the contrast checker: I was utilizing that tool in the docs and noticed the same thing you did. However I interpreted that as 14px font in the secondary or accent colors were not accessible on a white background. Which is why I added the makeHighContrastColor function to the typography file.

I agree with not wanting to move towards additional tokens for text color variants, and saw this as a middle ground.

My assumption with the above code would be that:

1. Plain ole text in the default euiColorSecondary color would not pass a11y
2. However <EuiText color="secondary"/> would pass because the computed value of the makeHighContrastColor is #00857D

I could be misunderstanding the code, though.

[cchaos]

Update: @daveyholler and I talked this one through explaining how the color checker works and components are using those colors. We've come up with a solution that will allow us to both use the colors as is but maintain a11y contrast. It does mean some breaks and more tokens (sorry @snide) but it'll all still be based off of those 4 main hex values that will cascade to these other tokens.

@daveyholler One thing we might want to do is, on the Colors Guidelines page, add a callout when switching to the the Amsterdam theme that indicates the breaks in a11y and the new vars.

The main point is that we're trying to create a compromise somewhere between keeping contrast and a11y acceptance with some more interesting visuals and aesthetics. My feeling is that even with the extra tokens, it's the same amount of maintainability as before. It just makes consuming applications have more effort to update.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2873/

[cchaos]

This PR is ready for final review 🎉

[cchaos]

And the summary has been updated to reflect the final changes.

[snide]

I haven't done a code review of this yet, but reading the summary, I'd suggest updating the the color and sass guidelines to document these new functions and variables.

[cchaos]

Yes, the colors guidelines page has been updated with a callout and shows the new variables (though only in Amsterdam-light mode).

As for the documenting the new function in the SASS guidelines, I want to re-do that page so we have more room to document more functions. At the moment, the new function will nor variables change the current theme's colors since they comply with the 4.5 contrast level.

[snide]

@cchaos I guess I wouldn't expect the docs to be only on the variant area and didn't expect to look for them there. They are available, and now in use in the current theme as well (even if their calculated color is the same). If they are items I can use, I'd just put them in all the docs and expose them without the toggle (keeping the explanation).

I know I'm a bit repetitive on this, but forcing our docs to show all the variables we're adding will help keep us thoughtful about how expensive (in terms of systems to learn) they are to add. It helps keep us honest 😄

Code looks pretty clean. Comments are mostly on docs.

[snide]

Even if we aren't adding this to the guidelines, I'd add some more explanation to why this is used. Assumption is someone will see this in use in the existing code, grep for the function and see that it calculates lower, but when is it a good time to use this over makeHighContrastColor. I know it sounds silly, but just say something like... "This is useful for icons so they don't get desaturated colors, but are still readable"...etc.

I'd also consider renaming it to makeGraphicContrastColor to better signal this is a contrast calculating function similar to makrHighContrastColor.

[cchaos]

I guess I wouldn't expect the docs to be only on the variant area and didn't expect to look for them there. They are available, and now in use in the current theme as well (even if their calculated color is the same).

I can understand this and will figure out a way to gracefully add this so it's visible for all themes.

If they are items I can use, I'd just put them in all the docs and expose them without the toggle (keeping the explanation).
I know I'm a bit repetitive on this, but forcing our docs to show all the variables we're adding will help keep us thoughtful about how expensive (in terms of systems to learn) they are to add. It helps keep us honest 😄

I'm not sure I agree with this part 100%. Yes, they are technically new variables, but they are calculated variables that just provides them a quicker way to get that contrast acceptable version without them having to remember the function name. The same as doing the calculation themselves:

$euiColorPrimaryText = makeHighContrastColor($euiColorPrimary)

Which is how were were handling downstream components, but we only documented makeHighContrastColor in a small nested section in the SASS guidelines. If we were to follow the same pattern, Colors guidelines would strictly detail the colors that the consumer can configure (manual hash colors), and the SASS docs would document the dynamic/calculated options.

Though I don't think they should be hidden down in the SASS guidelines, I do think it's acceptable to put these variants behind a toggle. We shouldn't explicitly list their calculated hash or rgb values like we do at the top with the main colors, but we do want to show how they are better suited for text in the Color contrast section.

However, if we list text and non-text versions, at the same time, this section becomes less optimal and more repetitive.

So I think the functionality of the toggle helps a lot, but I will work on the wording to make it appropriate for all themes.

[snide]

Feel free to merge when you get your docs sorted.

For a later PR I'd suggest simply making the variants more obvious for the copy / paste crowd. Ultimately I think that's my biggest concern. The old variables are less safe now, and knowing that, we should make them more obvious for people who are just looking for "green".

As we merge these PRs I think it'd be good to keep an upgrade log of what we'll need to do when we start using this theme. For this one it would be....

• "Check downstream usages of $euiColorX and make sure they now use $euiColorXText where appropriate`.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2873/

[cchaos]

1. I have updated the colors guidelines page to show the callout and text color variants for all themes.
2. Added an example of the makeGraphicColorContrast() mixin to the SASS guidelines.
3. @daveyholler Will start a meta ticket for compiling all the upgrade paths for consuming applications similar to this one: [META] Compressed panels release plan #2286 (comment)
4. I am working on an off-shoot branch to update our Colors guidelines page to better document the things we need

@thompsongl Do you mind doing a quick pass on the code for sanity?

[cchaos]

PR LGTM 😉

[cchaos]

Oh except we still need a Changelog. I will start one for this PR to establish a pattern.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2873/

[thompsongl]

Code changes LGTM

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2873/