-
Notifications
You must be signed in to change notification settings - Fork 1.1k
USWDS - Remove disabled classes. Resolves #5116 #5128
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
- File-input styles now use already included aria-disabled attribute as the selector for disabled styles. - Unit test updated to match this new pattern
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 so far. I added some comments for a couple items I saw.
I also wanted to flag that this work might conflict with the input prefix/suffix disabled fix: PR #5004. We should look into removing the --disabled classes from that fix as well.
Let me know if you have questions.
Tests performed:
- Confirm that all disabled-specific classes have been removed (wherever possible)
- Confirm that both
aria-disabledanddisabledtrigger disabled styles- Buttons
- File input
|
@amyleadem @mejiaj Restored |
amyleadem
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.
Looks good to me! Only thing I would consider is flagging this as a potentially breaking change in the PR description. We do advise against using the disabled button classes in our guidance, but it might be worth flagging just in case.
I performed the following tests:
- Confirm that all instances of disabled classes have been accounted for in this PR
- Confirm that all remaining instances of disabled classes are dynamically added (no additional burden on user)
- Confirm that none of the button variants rely on a disabled class
- Confirm that both
aria-disabledanddisabledshow disabled button styles -
npm run testcompletes without error
|
@mejiaj Bumping for review! |
|
@mahoneycm I've reviewed and approved. Please request re-review (if needed) via the the arrow on the reviewers panel. |
|
Work implemented in #5063 |
|
Removing this issue from 3.5.0 milestone to avoid confusion. Work was done in #5063. |
Summary
This PR takes a look at existing disabled classes and attempts to replace them by just using the
disabledandaria-disabledattributes in their place.In it's current state, USWDS contains two
--disabledclasses and one placeholder.usa-button--disabledclassThere was an existing
usa-button--disabledclass that acquired all of the same style definitions as button classes that had thedisabledoraria-disabledattributes. The classes themselves were not used on our storybook or USWDS Site.There is a note on site that states to not use the class unless for testing or debugging. It might be best to simply remove the class as well as the note on site.
usa-file-input--disabledclassFile input uses a class to apply disabled styling to the child elements of the target
<div>. Generally, this is acceptable behavior and helps to assign styles since the<div>element does not receive thedisabledattribute.After discussion, we have decided to allow disabled classes that style elements that:
This will allow us to correctly style elements as disabled without putting the burden on the user.
usa-date-picker--disabledplaceholderDate picker uses a disabled placeholder but applies these settings to elements with the
disabledoraria-disabledattributes, so we can safely ignore.Breaking change
This is potentially a breaking change
While we advise against using the
usa-button--disabledclass, this would break markdown for devs who have implemented itRelated issue
Closes #5116
Preview links
Button →
File Input (disabled) →
Problem statement
Rather than relying on specific state classes to target disabled components, we should instead target these elements using the
disabledoraria-disabledattributes.Solution
Testing 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:scssto format any Sass updates.npm testand confirm that all tests pass.Footnotes
https://designsystem.digital.gov/components/button/#:~:text=Use%20the%20browser%2Dnative%20disabled%20attribute%20for%20any%20disabled%20button.%20Don%E2%80%99t%20use%20usa%2Dbutton%2D%2Ddisabled%2C%20which%20is%20intended%20only%20for%20debugging%20and%20prototyping. ↩