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: add 4.5 contrast for text in partition slices #608

Merged
merged 78 commits into from
Jun 8, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
02f2f53
feat: move luminance value code from kibana in charts
rshen91 Mar 26, 2020
325b5bd
feat: textColor takes into account contrast with background slice
rshen91 Mar 31, 2020
6acca30
feat: account for initial non black or white textColor
rshen91 Mar 31, 2020
8f4ddf5
feat: add background color knob for xy charts
rshen91 Apr 10, 2020
23eb310
test: fix backgrounds to white for vrt
rshen91 Apr 13, 2020
d4b43ca
fix: add color background functionality to partition chart simple
rshen91 Apr 14, 2020
361200e
docs: add story for partition in styling
rshen91 Apr 14, 2020
963572b
feat: add combine color helper function
rshen91 Apr 15, 2020
01d1e2e
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 15, 2020
c7eec32
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 16, 2020
c36c79a
feat: add background to geometries
rshen91 Apr 20, 2020
fe5be74
fix: improve contrast
rshen91 Apr 20, 2020
cd31813
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 21, 2020
38e1250
fix: add updated story file
rshen91 Apr 21, 2020
69168b6
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 21, 2020
c1bc892
feat: update colorIsDark to adjust text for contrast
rshen91 Apr 21, 2020
7c3c7b0
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 22, 2020
69a2cc0
fix: readd textInvertible functionality in fill_text_layout
rshen91 Apr 22, 2020
7059318
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 22, 2020
f9a824d
WIP
rshen91 Apr 23, 2020
9bc19ba
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 23, 2020
65d8bc8
feat: improve makeHighContrastColor()
rshen91 Apr 23, 2020
71b512e
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 24, 2020
ba1b1cf
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 27, 2020
016cc7d
fix: merge conflicts
rshen91 Apr 27, 2020
9d0d6e7
WIP
rshen91 Apr 27, 2020
c0a56e0
feat: add link label contrast functionality
rshen91 Apr 27, 2020
878635c
chore: merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 28, 2020
94d69ec
test: update contrast logic and vrts
rshen91 Apr 28, 2020
1152d96
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 29, 2020
0688de9
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 29, 2020
e083865
fix: remove broken playground
rshen91 Apr 29, 2020
7ede9a3
fix: add playground from current master
rshen91 Apr 29, 2020
fc86b2a
test: add unit tests for calc file
rshen91 Apr 29, 2020
1fa7fb8
chore: merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Apr 30, 2020
08f1dd7
WIP
rshen91 Apr 30, 2020
a957653
WIP
rshen91 Apr 30, 2020
876062a
merging with master but broken build
rshen91 Apr 30, 2020
9492c23
WIP
rshen91 May 2, 2020
fcc7cfe
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 2, 2020
8035d6c
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 4, 2020
5b3b11f
feat: playground add foreground color functionality
rshen91 May 4, 2020
572cbdd
feat: error handling in conbineColors
rshen91 May 4, 2020
336ae5e
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 4, 2020
c09dfca
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 5, 2020
08768fa
feat: add contrastText prop and opacity in playground
rshen91 May 5, 2020
1800133
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 5, 2020
ee33cfd
fix: fix brightness lightness calc
rshen91 May 5, 2020
4f4801b
chore: merge commit
rshen91 May 8, 2020
7412a60
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 8, 2020
0a24a5e
feat: add textContrast number functionality
rshen91 May 11, 2020
1c04a77
feat: fix contrast with luminance over lightness for colorIsDark
rshen91 May 12, 2020
511b5b4
test: add unit tests and add class to playground
rshen91 May 13, 2020
48ec0fa
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 19, 2020
f8619e2
feat: low hanging fruit
rshen91 May 20, 2020
b411301
feat: add code review changes
rshen91 May 21, 2020
88e42d3
fix: add internal tag to LinkLabelsViewModelSpec
rshen91 May 21, 2020
41314f9
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 26, 2020
8a4c825
fix: cp tmp/charts.api.md to api/chart.api.md
rshen91 May 26, 2020
0946501
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 27, 2020
545e78e
feat: add chart background to own component
rshen91 May 27, 2020
ef78e4b
fix: fix order for <ChartBackground />
rshen91 May 27, 2020
299f90e
fix: add styling fix for legend
rshen91 May 28, 2020
aa48c17
chore: merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 28, 2020
69cd5d8
test: add unit test for combine colors and correct contrast
rshen91 May 28, 2020
462debe
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 May 28, 2020
01b7052
docs: add docs to storybook docs about textContrast
rshen91 May 29, 2020
9bb1e8e
Merge remote-tracking branch 'upstream/master' into luminance-pie
rshen91 Jun 1, 2020
36baf2e
feat: update baseThemes
rshen91 Jun 2, 2020
1864be2
Merge branch 'master' into luminance-pie
nickofthyme Jun 7, 2020
f14f2fa
fix: update pr with remaining checks
nickofthyme Jun 8, 2020
33aeb59
refactor: return types and rgb format
markov00 Jun 8, 2020
979ffe0
fix: get correct theme via selector and minor refactoring
markov00 Jun 8, 2020
af65d11
fix: refactor chart size
markov00 Jun 8, 2020
9012b54
fix: cleanup theme colors and add color check
markov00 Jun 8, 2020
20a11c0
fix: api docs
markov00 Jun 8, 2020
44e5101
Merge branch 'master' into luminance-pie
markov00 Jun 8, 2020
9c91e74
fix: broken tests, update prop naming
nickofthyme Jun 8, 2020
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
7 changes: 2 additions & 5 deletions api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,8 @@ export interface AxisStyle {
tickLabelPadding?: number;
}

// Warning: (ae-missing-release-tag) "BackgroundStyles" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export interface BackgroundStyles {
// (undocumented)
export interface BackgroundStyle {
color: string;
}

Expand Down Expand Up @@ -1411,7 +1408,7 @@ export interface Theme {
areaSeriesStyle: AreaSeriesStyle;
// (undocumented)
axes: AxisConfig;
background: BackgroundStyles;
background: BackgroundStyle;
barSeriesStyle: BarSeriesStyle;
bubbleSeriesStyle: BubbleSeriesStyle;
chartMargins: Margins;
Expand Down
31 changes: 23 additions & 8 deletions src/chart_types/partition_chart/layout/utils/calcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ export function arrayToLookup(keyFun: Function, array: Array<any>) {

/** If the user specifies the background of the container in which the chart will be on, we can use that color
* and make sure to provide optimal contrast
/** @internal */
markov00 marked this conversation as resolved.
Show resolved Hide resolved
export function combineColors(foregroundColor: Color, backgroundColor: Color) {
* @internal
*/
export function combineColors(foregroundColor: Color, backgroundColor: Color): Color {
const [red1, green1, blue1, alpha1] = chroma(foregroundColor).rgba();
const [red2, green2, blue2, alpha2] = chroma(backgroundColor).rgba();

Expand All @@ -63,23 +64,37 @@ export function combineColors(foregroundColor: Color, backgroundColor: Color) {
const combinedRed = Math.round((red1 * alpha1 + red2 * alpha2 * (1 - alpha1)) / combinedAlpha);
const combinedGreen = Math.round((green1 * alpha1 + green2 * alpha2 * (1 - alpha1)) / combinedAlpha);
const combinedBlue = Math.round((blue1 * alpha1 + blue2 * alpha2 * (1 - alpha1)) / combinedAlpha);
return RGBATupleToString([combinedRed, combinedGreen, combinedBlue, combinedAlpha] as RgbTuple);
const rgba: RgbTuple = [combinedRed, combinedGreen, combinedBlue, combinedAlpha];
return RGBATupleToString(rgba);
}

/**
* Returns a valid color
* @param color valid color
* @internal
*/
export function validateColor(color?: string, defaultColor?: string): string {
return color === undefined || color === 'transparent' ? defaultColor ?? 'rgba(255, 255, 255, 0)' : color;
export function validateColor(color?: string, defaultColor = 'rgba(255, 255, 255, 0)'): string {
return color === undefined || color === 'transparent' ? defaultColor : color;
}

/**
* Return true if the color is a valid CSS color, false otherwise
* @param color a color written in string
*/
export function isColorValid(color: string) {
try {
chroma(color);
return true;
} catch {
return false;
}
}

/**
* Adjust the text color in cases black and white can't reach ideal 4.5 ratio
* @internal
*/
export function makeHighContrastColor(foreground: Color, background: Color, ratio = 4.5) {
export function makeHighContrastColor(foreground: Color, background: Color, ratio = 4.5): Color {
// determine the lightness factor of the background color to determine whether to lighten or darken the foreground
const lightness = chroma(background).get('hsl.l');
let highContrastTextColor = foreground;
Expand Down Expand Up @@ -118,15 +133,15 @@ export function makeHighContrastColor(foreground: Color, background: Color, rati
* show contrast amount
* @internal
*/
export function getContrast(foregroundColor: string | chroma.Color, backgroundColor: string | chroma.Color) {
export function getContrast(foregroundColor: string | chroma.Color, backgroundColor: string | chroma.Color): number {
return chroma.contrast(foregroundColor, backgroundColor);
}

/**
* determines if the color is dark based on the luminance
* @internal
*/
export function colorIsDark(color: Color) {
export function colorIsDark(color: Color): boolean {
const luminance = chroma(color).luminance();
return luminance < 0.2;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function RGBATupleToString(rgba: RgbTuple): string {
if (rgba.length === 4) {
return `rgba(${rgba[0]}, ${rgba[1]}, ${rgba[2]}, ${rgba[3]})`;
}
return `rgba(${rgba[0]}, ${rgba[1]}, ${rgba[2]})`;
return `rgb(${rgba[0]}, ${rgba[1]}, ${rgba[2]})`;
}

/** convert rgb to hex
Expand Down
19 changes: 7 additions & 12 deletions src/chart_types/partition_chart/state/selectors/geometries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,18 @@ import { PartitionSpec } from '../../specs/index';
import { SpecTypes } from '../../../../specs/settings';
import { getTree } from './tree';
import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme';
import { isColorValid } from '../../layout/utils/calcs';

const getSpecs = (state: GlobalChartState) => state.specs;

/** @internal */
export const partitionGeometries = createCachedSelector(
[getSpecs, getChartContainerDimensionsSelector, getTree, getSettingsSpecSelector],
(specs, parentDimensions, tree, settingsSpec): ShapeViewModel => {
[getSpecs, getChartContainerDimensionsSelector, getTree, getChartThemeSelector],
(specs, parentDimensions, tree, theme): ShapeViewModel => {
const pieSpecs = getSpecsFromStore<PartitionSpec>(specs, ChartTypes.Partition, SpecTypes.Series);
// @ts-ignore
const { background } = settingsSpec.theme;
if (background !== undefined) {
return pieSpecs.length === 1
? render(pieSpecs[0], parentDimensions, tree, background.color)
: nullShapeViewModel();
} else {
return pieSpecs.length === 1 ? render(pieSpecs[0], parentDimensions, tree) : nullShapeViewModel();
}
const { color } = theme.background;
const bgColor: string | undefined = isColorValid(color) ? color : undefined;
return pieSpecs.length === 1 ? render(pieSpecs[0], parentDimensions, tree, bgColor) : nullShapeViewModel();
},
)((state) => state.chartId);
2 changes: 1 addition & 1 deletion src/components/_container.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.echChart {
position: relative;
display: flex;
height: 100%;
position: relative;

&--column {
flex-direction: column;
Expand Down
2 changes: 1 addition & 1 deletion src/components/_global.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
}

.echChartBackground {
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
position: absolute;
}
13 changes: 2 additions & 11 deletions src/components/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License. */

import React, { CSSProperties, createRef } from 'react';
import React, { createRef } from 'react';
import classNames from 'classnames';
import { Provider } from 'react-redux';
import { createStore, Store, Unsubscribe } from 'redux';
Expand Down Expand Up @@ -50,15 +50,6 @@ interface ChartState {
legendPosition: Position;
}

function getContainerStyle(size: any): CSSProperties {
if (size) {
return {
...getChartSize(size),
};
}
return {};
}

export class Chart extends React.Component<ChartProps, ChartState> {
static defaultProps: ChartProps = {
renderer: 'canvas',
Expand Down Expand Up @@ -156,7 +147,7 @@ export class Chart extends React.Component<ChartProps, ChartState> {

render() {
const { size, className } = this.props;
const containerStyle = getContainerStyle(size);
const containerStyle = getChartSize(size);
const horizontal = isHorizontalAxis(this.state.legendPosition);
const chartClassNames = classNames('echChart', className, {
'echChart--column': horizontal,
Expand Down
10 changes: 5 additions & 5 deletions src/components/chart_background.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
* under the License. */
import React from 'react';
import { connect } from 'react-redux';
import { BackgroundStyles } from '../utils/themes/theme';
import { getChartThemeSelector } from '../state/selectors/get_chart_theme';
import { GlobalChartState } from '../state/chart_state';
import { getInternalIsInitializedSelector } from '../state/selectors/get_internal_is_intialized';

interface ChartBackgroundProps {
backgroundStyles?: BackgroundStyles;
background: string;
}

export class ChartBackgroundComponent extends React.Component<ChartBackgroundProps> {
Expand All @@ -34,18 +33,19 @@ export class ChartBackgroundComponent extends React.Component<ChartBackgroundPro
}

render() {
return <div className="echChartBackground" style={{ background: this.props?.backgroundStyles?.color }} />;
const { background } = this.props;
return <div className="echChartBackground" style={{ background }} />;
Copy link
Collaborator

@nickofthyme nickofthyme Jun 8, 2020

Choose a reason for hiding this comment

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

This could allow non-colors to be passed to the background. Is this a good idea?

body {
  background: lightblue url("img_tree.gif") no-repeat fixed center;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

changed here 9c91e74

}
}

const mapStateToProps = (state: GlobalChartState): ChartBackgroundProps => {
if (!getInternalIsInitializedSelector(state)) {
return {
backgroundStyles: getChartThemeSelector(state).background,
background: 'transparent',
Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch

};
}
return {
backgroundStyles: getChartThemeSelector(state).background,
background: getChartThemeSelector(state).background.color,
};
};

Expand Down
8 changes: 4 additions & 4 deletions src/state/selectors/get_settings_specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import createCachedSelector from 're-reselect';
import { GlobalChartState } from '../chart_state';
import { ChartTypes } from '../../chart_types';
import { getSpecsFromStore } from '../utils';
import { SettingsSpec, SpecTypes } from '../../specs/settings';
import { SettingsSpec, SpecTypes, DEFAULT_SETTINGS_SPEC } from '../../specs/settings';
import { getChartIdSelector } from './get_chart_id';

const getSpecs = (state: GlobalChartState) => state.specs;
Expand All @@ -30,9 +30,9 @@ export const getSettingsSpecSelector = createCachedSelector(
[getSpecs],
(specs): SettingsSpec => {
const settingsSpecs = getSpecsFromStore<SettingsSpec>(specs, ChartTypes.Global, SpecTypes.Settings);
if (settingsSpecs.length > 1) {
throw new Error('Multiple settings specs are configured on the same chart');
if (settingsSpecs.length === 1) {
return settingsSpecs[0];
}
return settingsSpecs[0];
return DEFAULT_SETTINGS_SPEC;
},
)(getChartIdSelector);
7 changes: 1 addition & 6 deletions src/utils/chart_size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
* specific language governing permissions and limitations
* under the License. */

import { CSSProperties } from 'react';

export type ChartSizeArray = [number | string | undefined, number | string | undefined];
export interface ChartSizeObject {
width?: number | string;
Expand All @@ -27,26 +25,23 @@ export interface ChartSizeObject {
export type ChartSize = number | string | ChartSizeArray | ChartSizeObject;

/** @internal */
export function getChartSize(size?: ChartSize): CSSProperties {
export function getChartSize(size?: ChartSize): ChartSizeObject {
if (size === undefined) {
return {};
}
if (Array.isArray(size)) {
return {
position: 'relative',
width: size[0] === undefined ? '100%' : size[0],
height: size[1] === undefined ? '100%' : size[1],
};
} else if (typeof size === 'object') {
return {
position: 'relative',
width: size.width === undefined ? '100%' : size.width,
height: size.height === undefined ? '100%' : size.height,
};
}
const sameSize = size === undefined ? '100%' : size;
return {
position: 'relative',
width: sameSize,
height: sameSize,
};
Expand Down
2 changes: 1 addition & 1 deletion src/utils/themes/dark_theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,6 @@ export const DARK_THEME: Theme = {
},
},
background: {
color: 'transparent',
color: '#1D1E24', // $euiColorEmptyShade
},
};
2 changes: 1 addition & 1 deletion src/utils/themes/light_theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,6 @@ export const LIGHT_THEME: Theme = {
},
},
background: {
color: 'white',
color: '#FFFFFF', // $euiColorEmptyShade: #FFF !default;
},
};
9 changes: 7 additions & 2 deletions src/utils/themes/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,14 @@ export interface ColorConfig {
defaultVizColor: Color;
}
/**
* The background style applied to the chart.
* This is used to coordinate adequate contrast of the text in partition and treemap charts.
* @public
*/
export interface BackgroundStyles {
export interface BackgroundStyle {
/**
* The background color
*/
color: string;
}
export interface LegendStyle {
Expand Down Expand Up @@ -207,7 +212,7 @@ export interface Theme {
* The background allows the consumer to provide a color of the background container of the chart.
* This can then be used to calculate the contrast of the text for partition charts.
*/
background: BackgroundStyles;
background: BackgroundStyle;
}

export type PartialTheme = RecursivePartial<Theme>;
Expand Down