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(bar_chart): add shadow prop for value labels #785

Merged
merged 13 commits into from
Oct 13, 2020
Merged
10 changes: 9 additions & 1 deletion api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -535,9 +535,17 @@ export interface DisplayValueSpec {
}

// @public (undocumented)
export type DisplayValueStyle = TextStyle & {
export type DisplayValueStyle = Omit<TextStyle, 'fill'> & {
offsetX: number;
offsetY: number;
fill: Color | {
color: Color;
borderColor?: Color;
} | {
textInvertible: boolean;
textContrast?: number | boolean;
textBorder?: boolean;
};
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to add also the border width as a customizable option. This can help create different themes and adjust the theme when consuming the chart

};

// @public (undocumented)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions src/chart_types/xy_chart/renderer/canvas/primitives/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function renderText(
fontSize: number;
align: TextAlign;
baseline: TextBaseline;
shadow?: string;
},
degree: number = 0,
translation?: Partial<Point>,
Expand All @@ -49,6 +50,12 @@ export function renderText(
if (translation?.x || translation?.y) {
ctx.translate(translation?.x ?? 0, translation?.y ?? 0);
}
if (font.shadow) {
ctx.lineWidth = 1.5;
ctx.strokeStyle = font.shadow;
ctx.strokeText(text, origin.x, origin.y);
ctx.shadowBlur = 0;
Copy link
Member

Choose a reason for hiding this comment

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

shadowBlur is by default 0, probably you want to disable the shadowBlur if configured in the context somewhere else and probably before writing the stroke, so moving this before the strokeText call or before the if statement you want to apply that reset also to the fillText

}
ctx.fillText(text, origin.x, origin.y);
});
});
Expand Down
49 changes: 48 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/values/bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { BarGeometry } from '../../../../../utils/geometry';
import { Point } from '../../../../../utils/point';
import { Theme } from '../../../../../utils/themes/theme';
import { Font, FontStyle, TextBaseline, TextAlign } from '../../../../partition_chart/layout/types/types';
import { colorIsDark, getTextColorIfTextInvertible } from '../../../../partition_chart/layout/utils/calcs';
import { getFillTextColor } from '../../../../partition_chart/layout/viewmodel/fill_text_layout';
import { renderText, wrapLines } from '../primitives/text';
import { renderDebugRect } from '../utils/debug';

Expand Down Expand Up @@ -79,6 +81,7 @@ export function renderBarValues(ctx: CanvasRenderingContext2D, props: BarValuesP
}
const { width, height } = textLines;
const linesLength = textLines.lines.length;
const { fillColor, shadowColor } = getTextColors(fill, bars[i].color);

for (let i = 0; i < linesLength; i++) {
const text = textLines.lines[i];
Expand All @@ -89,10 +92,11 @@ export function renderBarValues(ctx: CanvasRenderingContext2D, props: BarValuesP
text,
{
...font,
fill,
fill: fillColor,
fontSize,
align,
baseline,
shadow: shadowColor,
},
-chartRotation,
);
Expand Down Expand Up @@ -200,3 +204,46 @@ function isOverflow(rect: Rect, chartDimensions: Dimensions, chartRotation: Rota

return false;
}

const DEFAULT_VALUE_COLOR = 'black';
// a little bit of alpha makes black font more readable
const DEFAULT_VALUE_BORDER_COLOR = 'rgba(255, 255, 255, 0.8)';
const TRANSPARENT_COLOR = 'rgba(0,0,0,0)';
type ValueFillDefinition = Theme['barSeriesStyle']['displayValue']['fill'];

function getTextColors(
fillDefinition: ValueFillDefinition,
geometryColor: string,
): { fillColor: string; shadowColor: string } {
if (typeof fillDefinition === 'string') {
return { fillColor: fillDefinition, shadowColor: TRANSPARENT_COLOR };
}
if ('color' in fillDefinition) {
return {
fillColor: fillDefinition.color,
shadowColor: fillDefinition.borderColor || TRANSPARENT_COLOR,
};
}
const fillColor =
getFillTextColor(
DEFAULT_VALUE_COLOR,
fillDefinition.textInvertible,
fillDefinition.textContrast || false,
geometryColor,
'white',
) || DEFAULT_VALUE_COLOR;

return {
fillColor,
shadowColor:
'textBorder' in fillDefinition
? getTextColorIfTextInvertible(
colorIsDark(fillColor),
colorIsDark(DEFAULT_VALUE_BORDER_COLOR),
DEFAULT_VALUE_BORDER_COLOR,
false,
geometryColor,
) || TRANSPARENT_COLOR
: TRANSPARENT_COLOR,
};
}
10 changes: 9 additions & 1 deletion src/utils/themes/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,17 @@ export interface Theme {
export type PartialTheme = RecursivePartial<Theme>;

/** @public */
export type DisplayValueStyle = TextStyle & {
export type DisplayValueStyle = Omit<TextStyle, 'fill'> & {
offsetX: number;
offsetY: number;
fill:
| Color
| { color: Color; borderColor?: Color }
| {
textInvertible: boolean;
textContrast?: number | boolean;
textBorder?: boolean;
};
Copy link
Member

Choose a reason for hiding this comment

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

This probably is a breaking change that needs to be notified

};

export interface PointStyle {
Expand Down
129 changes: 129 additions & 0 deletions stories/bar/50_label_value_advanced.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { boolean, color, number, select } from '@storybook/addon-knobs';
import React from 'react';

import { Axis, BarSeries, Chart, Position, ScaleType, Settings } from '../../src';
import { SeededDataGenerator } from '../../src/mocks/utils';
import { getChartRotationKnob } from '../utils/knobs';

const dataGen = new SeededDataGenerator();
function generateDataWithAdditional(num: number) {
return [...dataGen.generateSimpleSeries(num), { x: num, y: 0.25, g: 0 }, { x: num + 1, y: 8, g: 0 }];
}
const frozenDataSmallVolume = generateDataWithAdditional(10);
const frozenDataMediumVolume = generateDataWithAdditional(50);
const frozenDataHighVolume = generateDataWithAdditional(1500);

const frozenData: { [key: string]: any[] } = {
s: frozenDataSmallVolume,
m: frozenDataMediumVolume,
h: frozenDataHighVolume,
};

export const Example = () => {
const showValueLabel = boolean('show value label', true);
const isAlternatingValueLabel = boolean('alternating value label', false);
const isValueContainedInElement = boolean('contain value label within bar element', false);
const hideClippedValue = boolean('hide clipped value', false);

const displayValueSettings = {
showValueLabel,
isAlternatingValueLabel,
isValueContainedInElement,
hideClippedValue,
};

const debug = boolean('debug', false);
const useInverted = boolean('textInverted', false);

Choose a reason for hiding this comment

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

I was a bit surprised that the inverted colors aren't configurable in any way, was this intentional?

Copy link
Contributor Author

@dej611 dej611 Oct 13, 2020

Choose a reason for hiding this comment

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

Yes. There are basically three configurations available for DisplayValueLabels text:

// basic coloring
Color
|
// advanced coloring 
{
     color: Color;
     borderColor?: Color;
     borderWidth?: number;
   } 
|
// let the library compute it 
{
     textInvertible: boolean;
     textContrast?: number | boolean;
     textBorder?: number | boolean;
};

The only way to influence the library color pick in case of automatic color assignment is the textContrast parameter, which influence the threshold for the automatic switch between black and white.
This is consistent with other DisplayValueLabels APIs like the partitions one.

const valueColor = color('value color', '#fff');
const borderColor = color('value border color', 'rgba(0,0,0,1)');

const theme = {
barSeriesStyle: {
displayValue: {
fontSize: number('value font size', 10),
fontFamily: "'Open Sans', Helvetica, Arial, sans-serif",
fontStyle: 'normal',
padding: 0,
fill: useInverted
? { textInverted: useInverted, textContrast: true, textBorder: true }
: { color: valueColor, borderColor },
offsetX: number('offsetX', 0),
offsetY: number('offsetY', 0),
},
},
};

const dataSize = select(
'data volume size',
{
'small volume': 's',
'medium volume': 'm',
'high volume': 'h',
},
's',
);
const data = frozenData[dataSize];

const isSplitSeries = boolean('split series', false);
const isStackedSeries = boolean('stacked series', false);

const splitSeriesAccessors = isSplitSeries ? ['g'] : undefined;
const stackAccessors = isStackedSeries ? ['x'] : undefined;
return (
<Chart renderer="canvas" className="story-chart">
<Settings theme={theme} debug={debug} rotation={getChartRotationKnob()} showLegend showLegendExtra />
<Axis id="bottom" position={Position.Bottom} title="Bottom axis" showOverlappingTicks />
<Axis id="left2" title="Left axis" position={Position.Left} tickFormat={(d: any) => Number(d).toFixed(2)} />
<BarSeries
id="bars"
displayValueSettings={displayValueSettings}
xScaleType={ScaleType.Linear}
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={splitSeriesAccessors}
stackAccessors={stackAccessors}
data={data}
/>
<BarSeries
id="bars2"
displayValueSettings={displayValueSettings}
xScaleType={ScaleType.Linear}
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 2, g: 'a' },
{ x: 1, y: 7, g: 'a' },
{ x: 2, y: 3, g: 'a' },
{ x: 3, y: 6, g: 'a' },
{ x: 0, y: 4, g: 'b' },
{ x: 1, y: 5, g: 'b' },
{ x: 2, y: 8, g: 'b' },
{ x: 3, y: 2, g: 'b' },
]}
/>
</Chart>
);
};
1 change: 1 addition & 0 deletions stories/bar/bars.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export { Example as withLinearXAxisNoLinearInterval } from './6_linear_no_linear
export { Example as withTimeXAxis } from './7_with_time_xaxis';
export { Example as withLogYAxis } from './8_with_log_yaxis';
export { Example as withStackedLogYAxis } from './9_with_stacked_log';
export { Example as withValueLabelAdvanced } from './50_label_value_advanced';
Copy link
Member

Choose a reason for hiding this comment

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

can you move this right after the other withValueLabel so the user can play directly with the advanced settings?


export { Example as withAxisAndLegend } from './10_axis_and_legend';
export { Example as stackedWithAxisAndLegend } from './11_stacked_with_axis_and_legend';
Expand Down