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
11 changes: 10 additions & 1 deletion api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,18 @@ 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;
borderWidth?: number;
} | {
textInvertible: boolean;
textContrast?: number | boolean;
textBorder?: number | boolean;
};
alignment?: {
horizontal: Exclude<HorizontalAlignment, 'far' | 'near'>;
vertical: Exclude<VerticalAlignment, 'far' | 'near'>;
Expand Down
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.
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.
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: 1 addition & 1 deletion integration/tests/bar_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe('Bar series stories', () => {
HorizontalAlignment.Center,
HorizontalAlignment.Right,
])('Horizontal Alignment - %s', (horizontalAlignment) => {
const url = `http://localhost:9001/?path=/story/bar-chart--with-value-label&knob-chartRotation=${rotation}&knob-Horizontal alignment=${horizontalAlignment}&knob-Vertical alignment=${verticalAlignment}`;
const url = `http://localhost:9001/?path=/story/bar-chart--with-value-label-advanced&knob-chartRotation=${rotation}&knob-Horizontal alignment=${horizontalAlignment}&knob-Vertical alignment=${verticalAlignment}`;
it('place the value labels on the correct area', async () => {
await common.expectChartAtUrlToMatchScreenshot(url);
});
Expand Down
10 changes: 10 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,8 @@ export function renderText(
fontSize: number;
align: TextAlign;
baseline: TextBaseline;
shadow?: string;
shadowSize?: number;
},
degree: number = 0,
translation?: Partial<Point>,
Expand All @@ -49,6 +51,14 @@ export function renderText(
if (translation?.x || translation?.y) {
ctx.translate(translation?.x ?? 0, translation?.y ?? 0);
}
if (font.shadow) {
ctx.lineJoin = 'round';
const prevLineWidth = ctx.lineWidth;
ctx.lineWidth = font.shadowSize || 1.5;
ctx.strokeStyle = font.shadow;
ctx.strokeText(text, origin.x, origin.y);
ctx.lineWidth = prevLineWidth;
}
ctx.fillText(text, origin.x, origin.y);
});
});
Expand Down
73 changes: 72 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, TextAlignment } 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 @@ -87,6 +89,8 @@ export function renderBarValues(ctx: CanvasRenderingContext2D, props: BarValuesP
}
const { width, height } = textLines;
const linesLength = textLines.lines.length;
const shadowSize = getTextBorderSize(fill);
const { fillColor, shadowColor } = getTextColors(fill, bars[i].color, shadowSize);

for (let j = 0; j < linesLength; j++) {
const textLine = textLines.lines[j];
Expand All @@ -97,10 +101,12 @@ export function renderBarValues(ctx: CanvasRenderingContext2D, props: BarValuesP
textLine,
{
...font,
fill,
fill: fillColor,
fontSize,
align,
baseline,
shadow: shadowColor,
shadowSize,
},
-chartRotation,
);
Expand Down Expand Up @@ -343,3 +349,68 @@ 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 DEFAULT_VALUE_BORDER_SOLID_COLOR = 'rgb(255, 255, 255)';
const TRANSPARENT_COLOR = 'rgba(0,0,0,0)';
type ValueFillDefinition = Theme['barSeriesStyle']['displayValue']['fill'];

function getTextColors(
fillDefinition: ValueFillDefinition,
geometryColor: string,
borderSize: number,
): { 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;

// If the border is too wide it can overlap between a letter or another
// therefore use a solid color for thinker borders
const defaultBorderColor = borderSize < 2 ? DEFAULT_VALUE_BORDER_COLOR : DEFAULT_VALUE_BORDER_SOLID_COLOR;
const shadowColor =
'textBorder' in fillDefinition
? getTextColorIfTextInvertible(
colorIsDark(fillColor),
colorIsDark(defaultBorderColor),
defaultBorderColor,
false,
geometryColor,
) || TRANSPARENT_COLOR
: TRANSPARENT_COLOR;

return {
fillColor,
shadowColor,
};
}

const DEFAULT_BORDER_WIDTH = 1.5;
const MAX_BORDER_WIDTH = 8;

function getTextBorderSize(fill: ValueFillDefinition): number {
if (typeof fill === 'string') {
return DEFAULT_BORDER_WIDTH;
}
if ('borderWidth' in fill) {
return Math.min(fill.borderWidth || DEFAULT_BORDER_WIDTH, MAX_BORDER_WIDTH);
}
const borderWidth =
'textBorder' in fill && typeof fill.textBorder === 'number' ? fill.textBorder : DEFAULT_BORDER_WIDTH;
return Math.min(borderWidth, MAX_BORDER_WIDTH);
}
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; borderWidth?: number }
| {
textInvertible: boolean;
textContrast?: number | boolean;
textBorder?: number | boolean;
};
alignment?: {
horizontal: Exclude<HorizontalAlignment, 'far' | 'near'>;
vertical: Exclude<VerticalAlignment, 'far' | 'near'>;
Expand Down
22 changes: 0 additions & 22 deletions stories/bar/2_label_value.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,28 +63,6 @@ export const Example = () => {
fill: color('value color', '#000'),
Copy link
Contributor

@miukimiu miukimiu Sep 28, 2020

Choose a reason for hiding this comment

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

I've been testing and I think for the example white fill color looks better with a dark shadow:

Suggested change
fill: color('value color', '#000'),
fill: color('value color', '#fff'),

bars charts value@2x

Copy link
Contributor

@miukimiu miukimiu Sep 28, 2020

Choose a reason for hiding this comment

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

I also increased the size of the value font size to 12.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it should be a default setting or just a better starting point for this example?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was seeing the other charts where we have labels/values on top and normally they are black or with textInvertible: true

Screenshot 2020-09-28 at 14 42 08

So for consistency, I think we should keep the value labels black and just increase the size to 12.

How do you feel of then creating a second example to show that the value label can be customized? In this new example, you would use white value labels with a shadow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@miukimiu which would you prefer?

  1. Changing color contrast of the shadow
  2. Changing color contrast of the text itself
  3. Either of the above depending on if a shadow is enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding when it flips the color on text the shadow should change too.
But you are right, there is also the additional scenario where the shadow is not enabled (transparent?) and only the text changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah thats true but I was actually thinking that the shadow be null or undefined, as it's not needed when the text contrast is dynamic.

offsetX: number('offsetX', 0),
offsetY: number('offsetY', 0),
alignment: {
horizontal: select(
'Horizontal alignment',
{
Default: undefined,
Left: 'left',
Center: 'center',
Right: 'right',
},
undefined,
),
vertical: select(
'Vertical alignment',
{
Default: undefined,
Top: 'top',
Middle: 'middle',
Bottom: 'bottom',
},
undefined,
),
},
},
},
};
Expand Down
152 changes: 152 additions & 0 deletions stories/bar/51_label_value_advanced.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* 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);
const valueColor = color('value color', '#fff');
const borderColor = color('value border color', 'rgba(0,0,0,1)');
const borderSize = number('value border width', 1.5);

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: borderSize }
: { color: valueColor, borderColor, borderWidth: borderSize },
offsetX: number('offsetX', 0),
offsetY: number('offsetY', 0),
alignment: {
horizontal: select(
'Horizontal alignment',
{
Default: undefined,
Left: 'left',
Center: 'center',
Right: 'right',
},
undefined,
),
vertical: select(
'Vertical alignment',
{
Default: undefined,
Top: 'top',
Middle: 'middle',
Bottom: 'bottom',
},
undefined,
),
},
},
},
};

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 @@ -28,6 +28,7 @@ export default {

export { Example as basic } from './1_basic';
export { Example as withValueLabel } from './2_label_value';
export { Example as withValueLabelAdvanced } from './51_label_value_advanced';
export { Example as withAxis } from './3_with_axis';
export { Example as withOrdinalXAxis } from './4_ordinal';
export { Example as withLinearXAxis } from './5_linear';
Expand Down