-
Notifications
You must be signed in to change notification settings - Fork 1k
USWDS: Button - Forced colors outline #5147
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

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.
Hi @mahoneycm, thanks for taking this on. I like that you created a shared value for this. I had some concerns primarily about how this fix affects the focus state outline, so I added some questions in the comments.
Additionally, this might be out of scope, but I do not see a focus outline on usa-date-picker__button
in the forced colors mode. Just wanted to flag that for now.
Let me know if you have any questions.
packages/uswds-core/src/styles/mixins/utilities/_forced-color-outline.scss
Outdated
Show resolved
Hide resolved
- The original focus will add a thicker more defined focus outline

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 good! I found a couple visual changes and flagged them in comments. I also think we should move the $border-high-contrast
variable into another file. Let me know if you have questions!
packages/uswds-core/src/styles/settings/_settings-utilities.scss
Outdated
Show resolved
Hide resolved
packages/uswds-core/src/styles/mixins/helpers/checkbox-and-radio-colors.scss
Outdated
Show resolved
Hide resolved
packages/uswds-core/src/styles/mixins/helpers/checkbox-and-radio-colors.scss
Outdated
Show resolved
Hide resolved
- Moves and renames -high-contrast to -high-contrast-border - Switches HC range slider border to outline to resolve visual discrepency - Switches radio & checkbox border to outline to resolve visual discrepency - Removes extra line
@amyleadem Great catches! Changes have been implemented in 98005ed I believe range slider, radio, and checkbox were originally switched to borders when I was attempting to use a mixin for the high contrast border. Now that it's just a variable, we can have allow these to remain |
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.
Very close! I am happy this issue is getting fixed. I had just a couple notes about date picker and the settings page. Let me know if you have questions.
|
||
/* | ||
---------------------------------------- | ||
High Contrast |
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.
Wondering if it would be helpful to specify that this is for High contrast mode, possibly with a short description explaining HC mode/CSS forced-color mode is and how to activate it.
High Contrast | |
High contrast mode |
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.
Resolved in b2f35e6
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.
Approved. Thanks for all your work on this!
- We did this to prevent users from breaking contrast compliance - We may be able to move this back after providing high contrast documentation to users
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.
Smart fix to move this into /variables
. I like it. I really appreciate that you included the reasoning in the commit as well.
One small thing I did notice when I opened the preview is that the border on the disabled buttons disappears on hover when forced-colors are active. Would you be able to fix that here or open a fast-follow issue?
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.
Don't forget to run prettier. Added suggestion in comments, otherwise looks good.
packages/uswds-core/src/styles/mixins/helpers/checkbox-and-radio-colors.scss
Outdated
Show resolved
Hide resolved
@amyleadem It came from the
I removed it and it doesn't seem to change any styles outside of |
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.
Confirmed that removing border: 0
fixed the forced-colors issue and did not have a negative effect in the default presentation.
Confirmed in Chrome, Safari, Edge, Firefox.
I'll draw up a changelog PR for this change. |
Summary
This PR adds an outline to buttons in forced color mode for added clarity and consistency with default styles. It also replaces repetitive CSS with a new
$border-high-contrast
variableBreaking change
This is not a breaking change
Related issue
Closes #5099
Preview link
Buttons Preview →
Problem statement
Currently, buttons in forced-colors modes do not have an outline or any styling to appear like buttons. Instead, it just looks like regular text unless focused.
Solution
Define an outline within forced colors mode. Other components utilize similar styling in forced colors mode, so I've created a mixin and included in on
usa-button
Previously we set high-contrast outlines via
outline: 2px solid transparent
because it would not affect the border outside of high-contrast modes. Since we are using theforced-colors
media query for all high contrast styles, we can adjust the border without affecting components in normal circustances. Thus, high contrast borders have been turned into a sass variable and made uniform across our componentsTesting and review
Before opening this PR, make sure you’ve done whichever of these applies to you:
git pull origin [base branch]
to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop
).npm run prettier:scss
to format any Sass updates.