Skip to content
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

USWDS - Colors: Debug color contrast issues. #3949

Merged
merged 97 commits into from
Feb 5, 2021

Conversation

mejiaj
Copy link
Contributor

@mejiaj mejiaj commented Jan 15, 2021

Description

Fixes #3906 and #3926.

  • Adds sass-true for Mocha-based Sass unit testing (npm run test:sass)
  • Add standalone test script for selected Sass functions
  • Add options $context data to color-picker functions for clearer error messaging
  • Tried to address many edge cases, and test for them
  • Improves performance of color calculations
  • Improves error messaging
  • Demotes most color errors to warnings, with predictable fallbacks
  • Continues to throw errors for transparent color calculations
  • Adds special error handling to output some error messages as strings for testing purposes
  • Adds new helper functions for repeatable, single-purpose calculations:
    • is-accessible-magic-number(): Determines if two grades achieve a specified WCAG level
    • color-token-family(): Outputs the family of a color token or split
    • color-token-grade(): Outputs the grade (system or theme grade) of a color token or split. Note that this is different from calculate-grade() which will output the absolute system grade for any color.
    • color-token-type(): Outputs the type ("system" or "theme") of a color token
    • color-token-variant(): Outputs the variant ("vivid") of a color token or split
    • magic-number(): Outputs the magic number between two grades
    • next-token(): Gets the next color token "darker" or "lighter" from a specified color

@mejiaj mejiaj added skill: front end Affects: Accessibility 🟡 Relates to the accessibility of our components labels Jan 15, 2021
@@ -26,8 +26,9 @@
"release": "gulp release",
"spec": "npm run mocha -- 'spec/**/*.spec.js'",
"start": "npm run build && fractal start --sync & npm run watch",
"test": "snyk test && gulp eslint && gulp typecheck && gulp stylelint && fractal build && gulp test && gulp regression",
"test": "snyk test && gulp eslint && gulp typecheck && gulp stylelint && fractal build && gulp test && gulp regression && npm run test:sass",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the sass tests are not run as part of the main mocha job (gulp regression). I couldn't get it to work, but it works like this.

@@ -1,5 +1,4 @@
body {
background-color: color("white");
color: color("ink");
@include set-text-and-bg($context: "Body");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now body has user-settable colors.

$banner-guidance-measure: 3;
$banner-icon-colors: get-link-tokens-from-bg(
$theme-banner-background-color,
$theme-banner-link-color
$theme-banner-link-color,
$context: $banner-context
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that all these context vars help output better warning messages, to help users find the component settings that need updating

0: (
1,
1,
$system-color-grades: (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now with keys to make it more obvious. Also reversed the order to go from 100-0 so as you progress, the max of the current key is less than the min of the next key, making a more logical progression. It's strange because 100 is USWDS black but 1.00 luminance is white...

0,
);

$theme-color-grades: (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a similar map for system colors. Note that this one goes from lightest to darkest — again so the max of one token is less than the min of the next token.

);

$bg-color: if($bg-color == "default", get-default("bg-color"), $bg-color);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calculations are still a little ugly. I don't know how they could be improved

@import "../../../src/stylesheets/uswds.scss";

// is-system-color-token()
@include describe("[function] is-system-color-token()") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, some Sass unit tests. This is just a start. More to come on this front.

----------------------------------------
*/

$_error-output-override: false !default;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us to unit test error messaging without the error itself breaking the test run

----------------------------------------
*/

@function error-not-token($token, $type, $valid-token-map: false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to capture this common error in a repeatable function

----------------------------------------
*/

@function color-token-family($color-token) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and color-token-family() and color-token-grade()) can accept a token or a split. Split is faster!

@thisisdano thisisdano merged commit 07f12be into develop Feb 5, 2021
@thisisdano thisisdano deleted the jm-color-contrast-3906 branch February 5, 2021 17:57
@thisisdano thisisdano mentioned this pull request Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: Accessibility 🟡 Relates to the accessibility of our components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contrast calculation for identifier inaccurate
2 participants