-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 metric contrast #16296
Fix metric contrast #16296
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.
LGTM
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, but not sure why we're validating color string.
@@ -54,6 +56,14 @@ export class MetricVisComponent extends Component { | |||
return colors[label]; | |||
} | |||
|
|||
_needsLightText(bgColor) { | |||
const color = /rgb\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)/.exec(bgColor); |
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.
do we really need this? How does it arise that we don't know bgColor if of rgb()
format?
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.
The colors are currently calculated by the getHeatmapColors
function, which will always return a color in "rgb(...)" format to us.
This part will hopefully be cleaner once we build a proper palette color picker later on.
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.
Okay I guess I misread. We DO know that the color is in rgb()
format. That's why we parse it here, into it's red, green and blue component, so we can pass three numeric values to the isDarkColor
method.
* Update EUI to 0.0.14 * Make metrics text white when on dark color
Make use of the new
isColorDark
function in EUI to figure out, whether we should print light text for a metric, because the background color is dark.Fixes #13548
Partly adresses #15866 (point 2), by introducing proper contrast on any color