-
Notifications
You must be signed in to change notification settings - Fork 1.1k
USWDS: Fix disabled theme color settings #5402
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
Conversation
- This will prevent module loop errors
- Caused a Sass error because didn't follow color-grade format
|
Putting this in draft while I update the disabled color settings names. |
- Replace names to better align w/the naming convention in state colors
|
@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 👍 |
mejiaj
left a 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.
@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?
|
Great, thank you! |
…disabled-theme-colors
mahoneycm
left a 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.
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
|
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 Is it correct to say that if a project previously customized And if folks referenced |
|
@thisisdano Yes, that is a good clarification. |
Summary
$theme-color-disabled-lighterand$theme-color-disabled-darker.Breaking change
Related issue
Closes #5334
Related pull requests
Changelog PR
Preview link
Disabled form elements page
Problem statement
$theme-color-disabled-text,$theme-color-disabled-text-reverse, and$theme-color-text-on-disabledsettings return a module loop error for any custom configuration.$theme-color-successand$theme-color-error, the disabled color settings do not accept hex values.Solution
!defaultflag to these settings to make them configurable in projects.$theme-color-disabled-references in the Sass. Using shortcodes enables the system to accept hex code values for these settings.Settings updates
$theme-color-disabled-lighter$theme-color-disabled-light$theme-color-disabled-light$theme-color-disabled$theme-color-disabled$theme-color-disabled-dark$theme-color-disabled-dark$theme-color-disabled-darker$theme-color-disabled-text$theme-color-disabled]$theme-color-disabled-text-reverse$theme-color-disabled-light]$theme-color-text-on-disabled$theme-color-disabled-dark]Testing and review
$theme-color-disabled-references in Sass have been updated to shortcodesTo test configuration: