-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fix disabled text color on DescriptionListDescription #434
Conversation
Signed-off-by: Griffin-Sullivan <[email protected]>
50e1228
to
8c6765e
Compare
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.
/lgtm
Ok this is great, I would imagine this is gonna happen again soon, we should track this and make sure we port it in other places.
@lucferbux: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@lucferbux I added a comment to the PF6 SPIKE so people will be aware when swapping to PF6. |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign, lucferbux The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
In my last PR I was using
text.iconColorSubtle
to get around the disabled color being to low of a color ratio on a white background. It was causing our a11y testing to fail. Turns out you don't need to pass color contrast ratios on disabled elements so conditionally adding thearia-disabled
attribute will cause the tests to pass.The color change is probably not noticeable enough to anyone but can add an image if requested.
How Has This Been Tested?
Running cypress mock tests
Merge criteria:
DCO
check)If you have UI changes