Skip to content

Conversation

@amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Jul 26, 2023

Summary

  • Fixed configuration errors with disabled color settings. All disabled color settings should now be configurable and accept hex color values.
  • Updated disabled settings names to match other state colors. Updated default disabled color settings values and added $theme-color-disabled-lighter and $theme-color-disabled-darker.
  • Added "disabled-lighter" and "disabled-darker" color tokens. Also updated the default values for all disabled tokens.

Breaking change

⚠️ This is a breaking change. The names and values of disabled settings and tokens have changed and will need to be updated according to the table below if they are used in your project. All disabled states must meet minimum color contrast requirements for text (4.5:1).

Related issue

Closes #5334

Related pull requests

Changelog PR

Preview link

Disabled form elements page

Problem statement

  1. $theme-color-disabled-text, $theme-color-disabled-text-reverse, and $theme-color-text-on-disabled settings return a module loop error for any custom configuration.
$theme-color-disabled-light: "magenta-10", //compiles
$theme-color-disabled: "magenta-10", //compiles
$theme-color-disabled-dark: "magenta-10", //compiles
$theme-color-disabled-text: "magenta-10", //module loop error
$theme-color-disabled-text-reverse: "magenta-10", //module loop error
$theme-color-text-on-disabled: "magenta-10", //module loop error
  1. Unlike other state color settings like $theme-color-success and $theme-color-error, the disabled color settings do not accept hex values.
$theme-color-disabled-light: #fffff, //compiles (but not currently used in the system)
$theme-color-disabled: #333333, //error: not a token
$theme-color-disabled-dark: #333333, //compiles (but not currently used in the system)
$theme-color-disabled-text: #333333, //error: not a token
$theme-color-disabled-text-reverse: #333333, //error: not a token
$theme-color-text-on-disabled: #333333, //error: not a token

Solution

  1. Added the !default flag to these settings to make them configurable in projects.
  2. Created new disabled color shortcodes and used them to replace the $theme-color-disabled- references in the Sass. Using shortcodes enables the system to accept hex code values for these settings.
  3. Renamed the disabled color settings to match the convention in other state colors ("lighter", "light", "dark", etc). This adds flexibility to the use of these settings.

Settings updates

Previous setting name Previous default value New setting name New default value New token name
-- -- $theme-color-disabled-lighter "gray-20" "disabled-lighter"
$theme-color-disabled-light "gray-10" $theme-color-disabled-light "gray-40" "disabled-light"
$theme-color-disabled "gray-20" $theme-color-disabled "gray-50" "disabled"
$theme-color-disabled-dark "gray-30" $theme-color-disabled-dark "gray-70" "disabled-dark"
-- -- $theme-color-disabled-darker "gray-90" "disabled-darker"
$theme-color-disabled-text "gray-50" [Renamed to $theme-color-disabled]
$theme-color-disabled-text-reverse "gray-40" [Renamed to $theme-color-disabled-light]
$theme-color-text-on-disabled "gray-70" [Renamed to $theme-color-disabled-dark]

Warning
The settings do not create a warning when color combinations do not meet contrast standards. Recommend we open a new issue to address this.
Update: I've opened issue #5427 to address this.

Testing and review

  • Confirm that all disabled settings are configurable
  • Confirm that all disabled settings accept hex code values
  • Confirm that the updated settings names make sense and match convention
  • Confirm that all $theme-color-disabled- references in Sass have been updated to shortcodes
  • Confirm that all new tokens work in the color function
  • Confirm no visual regressions

To test configuration:

  1. In a test project, install this branch
  2. In your project, configure USWDS by adding hex values to the following settings. Example:
@use "uswds-core" with (
    $theme-color-disabled-lighter: #999999, 
    $theme-color-disabled-light: #888888, 
    $theme-color-disabled: #777777,
    $theme-color-disabled-dark: #555555,
    $theme-color-disabled-darker: #444444,
);
  1. Confirm that Sass compiles without errors.

@amyleadem amyleadem marked this pull request as ready for review July 27, 2023 21:43
- Caused a Sass error because didn't follow color-grade format
@amyleadem amyleadem requested review from mahoneycm and mejiaj July 28, 2023 16:40
@amyleadem
Copy link
Contributor Author

Putting this in draft while I update the disabled color settings names.

@amyleadem amyleadem marked this pull request as draft August 4, 2023 18:11
Amy Leadem added 2 commits August 4, 2023 11:33
- Replace names to better align w/the naming convention in state colors
@amyleadem amyleadem marked this pull request as ready for review August 4, 2023 19:31
@amyleadem
Copy link
Contributor Author

@mejiaj @mahoneycm re-opened this PR with new settings variable names. I still need to update the changelog/documentation PR, but the code should be ready for review 👍

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

@amyleadem nice fix. I was able to customize disabled color settings using gray-warm family.

Is there an issue created for this warning in PR description?

image

@amyleadem
Copy link
Contributor Author

@mejiaj I have opened #5427 to address the contrast warning concern.

@mejiaj
Copy link
Contributor

mejiaj commented Aug 7, 2023

Great, thank you!

Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Looking great!

  • Confirm that all disabled settings are configurable
  • Confirm that all disabled settings accept hex code values
  • Confirm that the updated settings names make sense and match convention
  • Confirm that all $theme-color-disabled- references in Sass have been updated to shortcodes
  • Confirm that all new tokens work in the color function
  • Confirm no visual regressions

@thisisdano
Copy link
Contributor

Trying to wrap my head around the breaking implications of this change.

If I look at this situation from the original issue:

@use 'uswds-core' with (
  $theme-color-disabled: #767676
);

After this work, this will no longer result in a compile error, but $theme-color-disabled no longer applies in the same way, internally.

Is it correct to say that if a project previously customized $theme-color-disabled they now should customize $theme-color-disabled-lighter since we now use that value for internal styling in the places we previously used $theme-color-disabled?

And if folks referenced $theme-color-disabled in custom Sass, they should update those references to $theme-color-disabled-lighter?

@amyleadem
Copy link
Contributor Author

@thisisdano Yes, that is a good clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

USWDS - Bug: Disabled color settings configuration errors

5 participants