Skip to content
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

feat(metric): allow alpha colors and improve contrast logic #2184

Merged
merged 23 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
48f2076
fix: hsl color logic to preserve alpha channel
nickofthyme Oct 2, 2023
ca3b11d
feat: allow alpha colors and improve contrast logic
nickofthyme Oct 2, 2023
0786046
fix: api changes
nickofthyme Oct 3, 2023
d42138b
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 3, 2023
29f198d
fix: pass text colors to contrast logic
nickofthyme Oct 4, 2023
a52acb3
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 4, 2023
f4cea4e
fix: bad vrt case [update-vrt]
nickofthyme Oct 5, 2023
65d4449
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 5, 2023
edf8e67
test: add vrt for transparent metric bg colors
nickofthyme Oct 5, 2023
50b5046
Merge branch 'main' into fix-metric-opacity
nickofthyme Oct 5, 2023
3d00b28
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 5, 2023
e7c9b93
chore: remove bg color check/error
nickofthyme Oct 5, 2023
8d7c1e2
test: fix theme params in url [update-vrt]
nickofthyme Oct 5, 2023
05859c3
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 5, 2023
290ab5b
Merge branch 'main' into fix-metric-opacity
nickofthyme Oct 6, 2023
6bd75a4
fix: split background and sparkline with alpha mask
nickofthyme Oct 6, 2023
ac205ac
Merge remote-tracking branch 'origin/fix-metric-opacity' into fix-met…
nickofthyme Oct 6, 2023
e254ac8
fix: add contrast check for text over sparkline
nickofthyme Oct 6, 2023
acde825
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Oct 6, 2023
c362e79
test: fix failing jest tests
nickofthyme Oct 10, 2023
e3b90fc
Merge branch 'main' into fix-metric-opacity
nickofthyme Oct 10, 2023
092a680
Merge remote-tracking branch 'origin/fix-metric-opacity' into fix-met…
nickofthyme Oct 10, 2023
9442cc7
Merge branch 'main' into fix-metric-opacity
nickofthyme Oct 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 0 additions & 2 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1840,8 +1840,6 @@ export type MetricSpecProps = ComponentProps<typeof Metric>;

// @public (undocumented)
export interface MetricStyle {
// (undocumented)
background: Color;
// (undocumented)
barBackground: Color;
// (undocumented)
Expand Down
24 changes: 21 additions & 3 deletions packages/charts/src/chart_types/metric/renderer/dom/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Metric as MetricComponent } from './metric';
import { highContrastColor } from '../../../../common/color_calcs';
import { colorToRgba } from '../../../../common/color_library_wrappers';
import { Colors } from '../../../../common/colors';
import { getResolvedBackgroundColor } from '../../../../common/fill_text_color';
import { BasicListener, ElementClickListener, ElementOverListener, settingsBuildProps } from '../../../../specs';
import { onChartRendered } from '../../../../state/actions/chart';
import { GlobalChartState } from '../../../../state/chart_state';
Expand All @@ -29,8 +30,9 @@ import {
import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme';
import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_spec';
import { Logger } from '../../../../utils/logger';
import { LIGHT_THEME } from '../../../../utils/themes/light_theme';
import { MetricStyle } from '../../../../utils/themes/theme';
import { BackgroundStyle, MetricStyle } from '../../../../utils/themes/theme';
import { MetricSpec } from '../../specs';
import { chartSize } from '../../state/selectors/chart_size';
import { getMetricSpecs } from '../../state/selectors/data';
Expand All @@ -47,6 +49,7 @@ interface StateProps {
specs: MetricSpec[];
a11y: A11ySettings;
style: MetricStyle;
background: BackgroundStyle;
locale: string;
onElementClick?: ElementClickListener;
onElementOut?: BasicListener;
Expand Down Expand Up @@ -76,6 +79,7 @@ class Component extends React.Component<StateProps & DispatchProps> {
a11y,
specs: [spec], // ignoring other specs
style,
background,
onElementClick,
onElementOut,
onElementOver,
Expand All @@ -94,8 +98,18 @@ class Component extends React.Component<StateProps & DispatchProps> {

const panel = { width: width / maxColumns, height: height / totalRows };

if (colorToRgba(background.color)[3] < 1) {
// TODO: centralize this check and bg color fallback across all chart types
Logger.expected(
'Text contrast requires an opaque background color, using fallbackColor blend',
'an opaque color',
background.color,
);
}
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved

const backgroundColor = getResolvedBackgroundColor(background.fallbackColor, background.color);
const emptyForegroundColor =
highContrastColor(colorToRgba(style.background)) === Colors.White.rgba
highContrastColor(colorToRgba(backgroundColor)) === Colors.White.rgba
? style.text.lightColor
: style.text.darkColor;

Expand Down Expand Up @@ -138,6 +152,7 @@ class Component extends React.Component<StateProps & DispatchProps> {
columnIndex={columnIndex}
panel={panel}
style={style}
backgroundColor={backgroundColor}
onElementClick={onElementClick}
onElementOut={onElementOut}
onElementOver={onElementOver}
Expand Down Expand Up @@ -185,6 +200,7 @@ const DEFAULT_PROPS: StateProps = {
},
a11y: DEFAULT_A11Y_SETTINGS,
style: LIGHT_THEME.metric,
background: LIGHT_THEME.background,
locale: settingsBuildProps.defaults.locale,
};

Expand All @@ -193,6 +209,7 @@ const mapStateToProps = (state: GlobalChartState): StateProps => {
return DEFAULT_PROPS;
}
const { onElementClick, onElementOut, onElementOver, locale } = getSettingsSpecSelector(state);
const { metric: style, background } = getChartThemeSelector(state);
return {
initialized: true,
chartId: state.chartId,
Expand All @@ -203,7 +220,8 @@ const mapStateToProps = (state: GlobalChartState): StateProps => {
onElementClick,
onElementOver,
onElementOut,
style: getChartThemeSelector(state).metric,
background,
style,
locale,
};
};
Expand Down
29 changes: 13 additions & 16 deletions packages/charts/src/chart_types/metric/renderer/dom/metric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import React, { CSSProperties, useState } from 'react';
import { ProgressBar } from './progress';
import { SparkLine } from './sparkline';
import { MetricText } from './text';
import { highContrastColor } from '../../../../common/color_calcs';
import { changeColorLightness, colorToRgba } from '../../../../common/color_library_wrappers';
import { Colors } from '../../../../common/colors';
import { changeColorLightness } from '../../../../common/color_library_wrappers';
import { Color } from '../../../../common/colors';
import { DEFAULT_CSS_CURSOR } from '../../../../common/constants';
import { fillTextColor } from '../../../../common/fill_text_color';
import {
BasicListener,
ElementClickListener,
Expand All @@ -39,6 +39,7 @@ export const Metric: React.FunctionComponent<{
datum: MetricDatum;
panel: Size;
style: MetricStyle;
backgroundColor: Color;
locale: string;
onElementClick?: ElementClickListener;
onElementOver?: ElementOverListener;
Expand All @@ -53,6 +54,7 @@ export const Metric: React.FunctionComponent<{
datum,
panel,
style,
backgroundColor,
locale,
onElementClick,
onElementOver,
Expand All @@ -75,34 +77,29 @@ export const Metric: React.FunctionComponent<{

const lightnessAmount = mouseState === 'leave' ? 0 : mouseState === 'enter' ? 0.05 : 0.1;
const interactionColor = changeColorLightness(datum.color, lightnessAmount, 0.8);
const backgroundInteractionColor = changeColorLightness(style.background, lightnessAmount, 0.8);

const datumWithInteractionColor: MetricDatum = {
...datum,
color: interactionColor,
};
const updatedStyle: MetricStyle = {
...style,
background: backgroundInteractionColor,
};

const event: MetricElementEvent = { type: 'metricElementEvent', rowIndex, columnIndex };

const containerStyle: CSSProperties = {
backgroundColor:
!isMetricWTrend(datumWithInteractionColor) && !isMetricWProgress(datumWithInteractionColor)
? datumWithInteractionColor.color
: updatedStyle.background,
: undefined,
cursor: onElementClick ? 'pointer' : DEFAULT_CSS_CURSOR,
borderColor: style.border,
};

const bgColor = isMetricWTrend(datum) || !isMetricWProgress(datum) ? datum.color : style.background;

const highContrastTextColor =
highContrastColor(colorToRgba(bgColor)) === Colors.White.rgba ? style.text.lightColor : style.text.darkColor;

const highContrastTextColor = fillTextColor(
backgroundColor,
isMetricWProgress(datum) ? backgroundColor : datum.color,
);
const onElementClickHandler = () => onElementClick && onElementClick([event]);

return (
// eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions,jsx-a11y/click-events-have-key-events
<div
Expand Down Expand Up @@ -142,14 +139,14 @@ export const Metric: React.FunctionComponent<{
id={metricHTMLId}
datum={datumWithInteractionColor}
panel={panel}
style={updatedStyle}
style={style}
onElementClick={onElementClick ? onElementClickHandler : undefined}
highContrastTextColor={highContrastTextColor}
locale={locale}
/>
{isMetricWTrend(datumWithInteractionColor) && <SparkLine id={metricHTMLId} datum={datumWithInteractionColor} />}
{isMetricWProgress(datumWithInteractionColor) && (
<ProgressBar datum={datumWithInteractionColor} barBackground={updatedStyle.barBackground} />
<ProgressBar datum={datumWithInteractionColor} barBackground={style.barBackground} />
)}
<div className="echMetric--outline" style={{ color: highContrastTextColor }}></div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export const SparkLine: FunctionComponent<{
trendShape === MetricTrendShape.Bars ? CurveType.CURVE_STEP_AFTER : CurveType.LINEAR,
);

const [h, s, l] = colorToHsl(color);
const pathColor = hslToColor(h, s, l >= 0.8 ? l - 0.1 : l + 0.1);
const [h, s, l, a] = colorToHsl(color);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have to blend first the color with a valid opaque background before changing the lightning, preserving the alpha is not the same thing as we where looking at doing here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what you're saying here, I need to blend the sparkline color with an opaque background?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right here since the transparent sparkline would be rendered on top of the transparent back area. I think a good fix of this is to just mask the filled sparkline from the background. See 6bd75a4

Screen Recording 2023-10-06 at 02 14 49 PM

const pathColor = hslToColor(h, s, l >= 0.8 ? l - 0.1 : l + 0.1, a);
const titleId = `${id}-trend-title`;
const descriptionId = `${id}-trend-description`;
return (
Expand Down
16 changes: 9 additions & 7 deletions packages/charts/src/common/color_library_wrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,20 @@ export function colorToRgba(color: Color): RgbaTuple {
}

/** @internal */
export function colorToHsl(color: Color) {
const [r, g, b] = colorToRgba(color);
return chroma.rgb(r, g, b).hsl();
export function colorToHsl(color: Color): [h: number, s: number, l: number, a: number] {
const [r, g, b, a] = colorToRgba(color);
const [h, s, l] = chroma.rgb(r, g, b).hsl(); // alpha not preserved
return [h, s, l, a];
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
}

/** @internal */
export function hslToColor(h: number, s: number, l: number): Color {
const rgba = chroma.hsl(h, s, l).rgba();
export function hslToColor(h: number, s: number, l: number, a = 1): Color {
const rgba = chroma.hsl(h, s, l).alpha(a).rgba();
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
return RGBATupleToString(rgba);
}

/** @internal */
export function changeColorLightness(color: Color, lightnessAmount: number, lightnessThreshold: number): Color {
const [h, s, l] = colorToHsl(color);
return hslToColor(h, s, l >= lightnessThreshold ? l - lightnessAmount : l + lightnessAmount);
const [h, s, l, a] = colorToHsl(color);
return hslToColor(h, s, l >= lightnessThreshold ? l - lightnessAmount : l + lightnessAmount, a);
}
14 changes: 14 additions & 0 deletions packages/charts/src/common/fill_text_color.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,17 @@ export function fillTextColor(

return RGBATupleToString(highContrastColor(backgroundRGBA));
}

/** @internal */
export function getResolvedBackgroundColor(
fallbackBGColor: Color,
background: Color = Colors.Transparent.keyword,
): Color {
let backgroundRGBA = colorToRgba(background);

if (backgroundRGBA[3] < TRANSPARENT_LIMIT) {
backgroundRGBA = colorToRgba(fallbackBGColor);
}

return RGBATupleToString(backgroundRGBA);
}
1 change: 0 additions & 1 deletion packages/charts/src/utils/themes/dark_theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ export const DARK_THEME: Theme = {
},
border: '#343741',
barBackground: '#343741',
background: '#1D1E23',
nonFiniteText: 'N/A',
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
minHeight: 64,
},
Expand Down
1 change: 0 additions & 1 deletion packages/charts/src/utils/themes/light_theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,6 @@ export const LIGHT_THEME: Theme = {
},
border: '#EDF0F5',
barBackground: '#EDF0F5',
background: '#FFFFFF',
nonFiniteText: 'N/A',
minHeight: 64,
},
Expand Down
1 change: 0 additions & 1 deletion packages/charts/src/utils/themes/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ export interface MetricStyle {
lightColor: Color;
};
border: Color;
background: Color;
barBackground: Color;
nonFiniteText: string;
minHeight: Pixels;
Expand Down