-
Notifications
You must be signed in to change notification settings - Fork 5
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(TabInfoButton): [EMU-7334] Increase contrast #309
Conversation
@@ -25,7 +25,7 @@ | |||
} | |||
|
|||
svg { | |||
fill: $ds-primary-100; | |||
fill: $ds-grey-400; |
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.
Not necessary as part of this PR but if you want, you can also replace this inline SVG with
<InfoInfo color="grey-400" size={20 /* or other */} />
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.
Sounds nifty, but how would I then give it hover states and similar? I tried manipulating it with
svg {
fill: SOMECOLOR
}
to no avail :/
Looks like the fill is set to "currentColor" at all times
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.
Nvm, I need to use svg -> path instead.
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.
You can then remove the svg fill color as the Icon component captures the button color (that's what currentColor is doing):
.button {
color: $ds-primary-400;
&:hover {
color: $ds-primary-300;
}
}
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.
Ah cool 👍 Will keep that in mind and use it in the future! Here however, for better a11y, I wrapped the icon in a real button and just tweaked the original custom css instead of creating 2 new classes (one for the wrapping button and one for the icon itself).
ee6a8c6
to
7010599
Compare
What this PR does
This PR increases the contrast ratio between TabInfoButton and background as user testing showed that it's really hard to see.
Before
After
Why is this needed?
Solves:
EMU-7334
Checklist:
Or give a reason why this does not apply:
Or give a reason why this does not apply:
Browser support
My code works in the following browsers: