From 29c13ebc910f196967635a3becd101048cd824ff Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2019 15:13:36 -0500 Subject: [PATCH 1/5] fix(renderer): stroke opacity --- .../xy_chart/rendering/rendering.ts | 22 ++++---- .../react_canvas/bar_geometries.tsx | 36 +++++++------ .../utils/rendering_props_utils.ts | 52 ++++++++++++++++--- src/utils/themes/theme.ts | 16 ++++-- 4 files changed, 93 insertions(+), 33 deletions(-) diff --git a/src/chart_types/xy_chart/rendering/rendering.ts b/src/chart_types/xy_chart/rendering/rendering.ts index 9f172cb13f..8a68c7da95 100644 --- a/src/chart_types/xy_chart/rendering/rendering.ts +++ b/src/chart_types/xy_chart/rendering/rendering.ts @@ -34,6 +34,7 @@ export interface GeometryValue { /** Shared style properties for varies geometries */ export interface GeometryStyle { opacity: number; + strokeOpacity?: number; } export type IndexedGeometry = PointGeometry | BarGeometry; @@ -483,17 +484,20 @@ export function getGeometryStyle( geometryId: GeometryId, highlightedLegendItem: LegendItem | null, sharedThemeStyle: SharedGeometryStyle, - specOpacity?: number, + opacity?: number, + strokeOpacity?: number, individualHighlight?: { [key: string]: boolean }, ): GeometryStyle { - const sharedStyle = - specOpacity == null - ? sharedThemeStyle - : { - ...sharedThemeStyle, - highlighted: { opacity: specOpacity }, - default: { opacity: specOpacity }, - }; + const base = { opacity, strokeOpacity }; + const sharedStyle = mergePartial(sharedThemeStyle, { + default: base, + highlighted: base, + unhighlighted: { + strokeOpacity: strokeOpacity + ? (strokeOpacity + sharedThemeStyle.unhighlighted.opacity) / 2 + : sharedThemeStyle.unhighlighted.opacity, + }, + }); if (highlightedLegendItem != null) { const isPartOfHighlightedSeries = belongsToDataSeries(geometryId, highlightedLegendItem.value); diff --git a/src/components/react_canvas/bar_geometries.tsx b/src/components/react_canvas/bar_geometries.tsx index a0b7491618..013b8fc8b2 100644 --- a/src/components/react_canvas/bar_geometries.tsx +++ b/src/components/react_canvas/bar_geometries.tsx @@ -5,7 +5,7 @@ import { animated, Spring } from 'react-spring/renderprops-konva.cjs'; import { LegendItem } from '../../chart_types/xy_chart/legend/legend'; import { BarGeometry, getGeometryStyle } from '../../chart_types/xy_chart/rendering/rendering'; import { SharedGeometryStyle } from '../../utils/themes/theme'; -import { buildBarRenderProps } from './utils/rendering_props_utils'; +import { buildBarRenderProps, buildBarBorderRenderProps } from './utils/rendering_props_utils'; interface BarGeometriesDataProps { animated?: boolean; @@ -56,6 +56,7 @@ export class BarGeometries extends React.PureComponent {(props: { y: number; height: number }) => { + const barPropsBorder = buildBarBorderRenderProps( + x, + props.y, + width, + props.height, + seriesStyle.rectBorder, + geometryStyle, + ); const barProps = buildBarRenderProps( x, props.y, @@ -72,29 +81,26 @@ export class BarGeometries extends React.PureComponent; + return ( + + + {barPropsBorder && } + + ); }} ); } else { - const barProps = buildBarRenderProps( - x, - y, - width, - height, - color, - seriesStyle.rect, - seriesStyle.rectBorder, - geometryStyle, - ); + const barPropsBorder = buildBarBorderRenderProps(x, y, width, height, seriesStyle.rectBorder, geometryStyle); + const barProps = buildBarRenderProps(x, y, width, height, color, seriesStyle.rect, geometryStyle); return ( - - + + + {barPropsBorder && } ); } diff --git a/src/components/react_canvas/utils/rendering_props_utils.ts b/src/components/react_canvas/utils/rendering_props_utils.ts index 745712a113..55c491fc96 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.ts @@ -10,6 +10,7 @@ import { } from '../../../utils/themes/theme'; import { Dimensions } from '../../../utils/dimensions'; import { GlobalKonvaElementProps } from '../globals'; +import { RectConfig } from 'konva'; export interface PointStyleProps { radius: number; @@ -338,7 +339,6 @@ export function buildAreaRenderProps( * @param height the height of the rect * @param color the computed color of the rect for this series * @param rectStyle the rect style - * @param borderStyle the border rect style * @param geometryStyle the highlight geometry style */ export function buildBarRenderProps( @@ -348,19 +348,59 @@ export function buildBarRenderProps( height: number, color: string, rectStyle: RectStyle, - borderStyle: RectBorderStyle, geometryStyle: GeometryStyle, -) { +): RectConfig { return { x, y, width, height, fill: rectStyle.fill || color, - strokeWidth: borderStyle.strokeWidth, - stroke: borderStyle.stroke || 'transparent', - strokeEnabled: borderStyle.visible && borderStyle.strokeWidth > 0, + strokeEnabled: false, + ...geometryStyle, + ...GlobalKonvaElementProps, + }; +} + +/** + * Return the rendering props for a bar. The color of the bar will be overwritten + * by the fill color of the rectStyle parameter if present + * @param x the x position of the rect + * @param y the y position of the rect + * @param width the width of the rect + * @param height the height of the rect + * @param color the computed color of the rect for this series + * @param rectStyle the rect style + * @param borderStyle the border rect style + * @param geometryStyle the highlight geometry style + */ +export function buildBarBorderRenderProps( + x: number, + y: number, + width: number, + height: number, + borderStyle: RectBorderStyle, + geometryStyle: GeometryStyle, +): RectConfig | null { + const { stroke, visible, strokeWidth } = borderStyle; + + if (!visible || strokeWidth <= 0 || !stroke) { + return null; + } + + const opacity = geometryStyle.strokeOpacity; + + return { + x: x + strokeWidth / 2, + y: y + strokeWidth / 2, + width: width - strokeWidth, + height: height - strokeWidth, + fillEnabled: false, + strokeEnabled: true, + strokeWidth, + stroke, ...geometryStyle, + opacity, // want to override opactiy of geometryStyle ...GlobalKonvaElementProps, }; } diff --git a/src/utils/themes/theme.ts b/src/utils/themes/theme.ts index 827f047b6f..58e284c820 100644 --- a/src/utils/themes/theme.ts +++ b/src/utils/themes/theme.ts @@ -164,12 +164,22 @@ export interface RectStyle { } export interface RectBorderStyle { - /** is the rect border visible or hidden ? */ + /** + * Border visibility + */ visible: boolean; - /** a static stroke color if defined, if not it will use the color of the series */ + /** + * Border stroke color + */ stroke?: string; - /** the stroke width of the rect border */ + /** + * Border stroke width + */ strokeWidth: number; + /** + * Border stroke opacity + */ + strokeOpacity?: number; } export interface BarSeriesStyle { rect: RectStyle; From 6889cc0285510ec2d7bef5c4ae0bf783c7d56e04 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2019 12:33:15 -0500 Subject: [PATCH 2/5] refactor: shared opacity to be an opacity multiplication factor --- .playground/playgroud.tsx | 86 ++++--------------- .../xy_chart/rendering/rendering.ts | 29 +++---- .../react_canvas/area_geometries.tsx | 21 +---- .../react_canvas/bar_geometries.tsx | 14 ++- .../react_canvas/line_geometries.tsx | 6 +- .../utils/rendering_props_utils.ts | 27 ++++-- src/utils/themes/theme.ts | 4 +- 7 files changed, 68 insertions(+), 119 deletions(-) diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index 759a5c8e8c..4719c9efd6 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -1,16 +1,6 @@ import React from 'react'; -import { - Axis, - Chart, - getAxisId, - getSpecId, - niceTimeFormatter, - Position, - ScaleType, - Settings, - LineSeries, -} from '../src'; +import { Axis, Chart, getAxisId, getSpecId, niceTimeFormatter, Position, ScaleType, Settings, BarSeries } from '../src'; import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana'; import { CursorEvent } from '../src/specs/settings'; import { CursorUpdateListener } from '../src/chart_types/xy_chart/store/chart_state'; @@ -28,44 +18,8 @@ export class Playground extends React.Component { render() { return ( - <> - {renderChart( - '1', - this.ref1, - KIBANA_METRICS.metrics.kibana_os_load[0].data.slice(0, 15), - this.onCursorUpdate, - true, - )} - {renderChart( - '2', - this.ref2, - KIBANA_METRICS.metrics.kibana_os_load[1].data.slice(0, 15), - this.onCursorUpdate, - true, - )} - {renderChart('3', this.ref3, KIBANA_METRICS.metrics.kibana_os_load[1].data.slice(15, 30), this.onCursorUpdate)} - - ); - } -} - -function renderChart( - key: string, - ref: React.RefObject, - data: any, - onCursorUpdate?: CursorUpdateListener, - timeSeries: boolean = false, -) { - return ( -
- - + + d.toFixed(2)} /> - - - -
- ); + ); + } } diff --git a/src/chart_types/xy_chart/rendering/rendering.ts b/src/chart_types/xy_chart/rendering/rendering.ts index 8a68c7da95..3a23fa3340 100644 --- a/src/chart_types/xy_chart/rendering/rendering.ts +++ b/src/chart_types/xy_chart/rendering/rendering.ts @@ -33,8 +33,12 @@ export interface GeometryValue { /** Shared style properties for varies geometries */ export interface GeometryStyle { + /** + * Opacity multiplier + * + * if set to `0.5` all given opacities will be halfed + */ opacity: number; - strokeOpacity?: number; } export type IndexedGeometry = PointGeometry | BarGeometry; @@ -483,37 +487,26 @@ export function renderArea( export function getGeometryStyle( geometryId: GeometryId, highlightedLegendItem: LegendItem | null, - sharedThemeStyle: SharedGeometryStyle, - opacity?: number, - strokeOpacity?: number, + sharedGeometryStyle: SharedGeometryStyle, individualHighlight?: { [key: string]: boolean }, ): GeometryStyle { - const base = { opacity, strokeOpacity }; - const sharedStyle = mergePartial(sharedThemeStyle, { - default: base, - highlighted: base, - unhighlighted: { - strokeOpacity: strokeOpacity - ? (strokeOpacity + sharedThemeStyle.unhighlighted.opacity) / 2 - : sharedThemeStyle.unhighlighted.opacity, - }, - }); + const { default: defaultStyles, highlighted, unhighlighted } = sharedGeometryStyle; if (highlightedLegendItem != null) { const isPartOfHighlightedSeries = belongsToDataSeries(geometryId, highlightedLegendItem.value); - return isPartOfHighlightedSeries ? sharedStyle.highlighted : sharedStyle.unhighlighted; + return isPartOfHighlightedSeries ? highlighted : unhighlighted; } if (individualHighlight) { const { hasHighlight, hasGeometryHover } = individualHighlight; if (!hasGeometryHover) { - return sharedStyle.highlighted; + return highlighted; } - return hasHighlight ? sharedStyle.highlighted : sharedStyle.unhighlighted; + return hasHighlight ? highlighted : unhighlighted; } - return sharedStyle.default; + return defaultStyles; } export function isPointOnGeometry( diff --git a/src/components/react_canvas/area_geometries.tsx b/src/components/react_canvas/area_geometries.tsx index 86350dc288..595fb02ce6 100644 --- a/src/components/react_canvas/area_geometries.tsx +++ b/src/components/react_canvas/area_geometries.tsx @@ -90,13 +90,7 @@ export class AreaGeometries extends React.PureComponent { const { seriesPointStyle, geometryId } = glyph; if (seriesPointStyle.visible) { - const customOpacity = seriesPointStyle ? seriesPointStyle.opacity : undefined; - const geometryStyle = getGeometryStyle( - geometryId, - this.props.highlightedLegendItem, - sharedStyle, - customOpacity, - ); + const geometryStyle = getGeometryStyle(geometryId, this.props.highlightedLegendItem, sharedStyle); const pointStyleProps = buildPointStyleProps(glyph.color, seriesPointStyle, geometryStyle); elements.push(...this.renderPoints(glyph.points, i, pointStyleProps, glyph.geometryId)); } @@ -117,13 +111,7 @@ export class AreaGeometries extends React.PureComponent { const { area, color, transform, geometryId, seriesAreaStyle } = glyph; - const customOpacity = seriesAreaStyle ? seriesAreaStyle.opacity : undefined; - const geometryStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedStyle, customOpacity); + const geometryStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedStyle); const key = getGeometryIdKey(geometryId, 'area-'); const areaProps = buildAreaRenderProps(transform.x, area, color, seriesAreaStyle, geometryStyle); return ; @@ -149,7 +136,7 @@ export class AreaGeometries extends React.PureComponent { const { lines, color, geometryId, transform, seriesAreaLineStyle } = glyph; - const geometryStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedStyle, seriesAreaLineStyle.opacity); + const geometryStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedStyle); return lines.map((linePath, lineIndex) => { const key = getGeometryIdKey(geometryId, `area-line-${areaIndex}-${lineIndex}`); diff --git a/src/components/react_canvas/bar_geometries.tsx b/src/components/react_canvas/bar_geometries.tsx index 013b8fc8b2..4e2f424a20 100644 --- a/src/components/react_canvas/bar_geometries.tsx +++ b/src/components/react_canvas/bar_geometries.tsx @@ -55,8 +55,6 @@ export class BarGeometries extends React.PureComponent ); } else { - const barPropsBorder = buildBarBorderRenderProps(x, y, width, height, seriesStyle.rectBorder, geometryStyle); + const barPropsBorder = buildBarBorderRenderProps( + x, + y, + width, + height, + seriesStyle.rect, + seriesStyle.rectBorder, + geometryStyle, + ); const barProps = buildBarRenderProps(x, y, width, height, color, seriesStyle.rect, geometryStyle); + return ( diff --git a/src/components/react_canvas/line_geometries.tsx b/src/components/react_canvas/line_geometries.tsx index daccc8e79f..abcd60cce9 100644 --- a/src/components/react_canvas/line_geometries.tsx +++ b/src/components/react_canvas/line_geometries.tsx @@ -80,16 +80,14 @@ export class LineGeometries extends React.PureComponent; } getPointToRender(glyph: LineGeometry, sharedStyle: SharedGeometryStyle, key: string) { const { points, color, geometryId, seriesPointStyle } = glyph; - const customOpacity = seriesPointStyle ? seriesPointStyle.opacity : undefined; - const geometryStyle = getGeometryStyle(geometryId, this.props.highlightedLegendItem, sharedStyle, customOpacity); + const geometryStyle = getGeometryStyle(geometryId, this.props.highlightedLegendItem, sharedStyle); const pointStyleProps = buildPointStyleProps(color, seriesPointStyle, geometryStyle); return this.renderPoints(points, key, pointStyleProps); } diff --git a/src/components/react_canvas/utils/rendering_props_utils.ts b/src/components/react_canvas/utils/rendering_props_utils.ts index 55c491fc96..9c7a472de3 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.ts @@ -10,7 +10,7 @@ import { } from '../../../utils/themes/theme'; import { Dimensions } from '../../../utils/dimensions'; import { GlobalKonvaElementProps } from '../globals'; -import { RectConfig } from 'konva'; +import { RectConfig, PathConfig, CircleConfig } from 'konva'; export interface PointStyleProps { radius: number; @@ -255,8 +255,8 @@ export function buildPointStyleProps( strokeWidth, strokeEnabled: strokeWidth !== 0, fill: fill, - opacity, ...geometryStyle, + opacity: opacity * geometryStyle.opacity, }; } @@ -266,7 +266,7 @@ export function buildPointStyleProps( * @param y the y position of the point * @param pointStyleProps the style props of the point */ -export function buildPointRenderProps(x: number, y: number, pointStyleProps: PointStyleProps) { +export function buildPointRenderProps(x: number, y: number, pointStyleProps: PointStyleProps): CircleConfig { return { x, y, @@ -290,7 +290,9 @@ export function buildLineRenderProps( color: string, lineStyle: LineStyle, geometryStyle: GeometryStyle, -) { +): PathConfig { + const opacity = lineStyle.opacity * geometryStyle.opacity; + return { x, data: linePath, @@ -299,6 +301,7 @@ export function buildLineRenderProps( lineCap: 'round', lineJoin: 'round', ...geometryStyle, + opacity, // want to override opactiy of geometryStyle ...GlobalKonvaElementProps, }; } @@ -318,7 +321,9 @@ export function buildAreaRenderProps( color: string, areaStyle: AreaStyle, geometryStyle: GeometryStyle, -) { +): PathConfig { + const opacity = areaStyle.opacity * geometryStyle.opacity; + return { x: xTransform, data: areaPath, @@ -326,6 +331,7 @@ export function buildAreaRenderProps( lineCap: 'round', lineJoin: 'round', ...geometryStyle, + opacity, // want to override opactiy of geometryStyle ...GlobalKonvaElementProps, }; } @@ -350,6 +356,8 @@ export function buildBarRenderProps( rectStyle: RectStyle, geometryStyle: GeometryStyle, ): RectConfig { + const opacity = rectStyle.opacity * geometryStyle.opacity; + return { x, y, @@ -358,6 +366,7 @@ export function buildBarRenderProps( fill: rectStyle.fill || color, strokeEnabled: false, ...geometryStyle, + opacity, // want to override opactiy of geometryStyle ...GlobalKonvaElementProps, }; } @@ -379,17 +388,17 @@ export function buildBarBorderRenderProps( y: number, width: number, height: number, + barStyle: RectStyle, borderStyle: RectBorderStyle, geometryStyle: GeometryStyle, ): RectConfig | null { - const { stroke, visible, strokeWidth } = borderStyle; + const { stroke, visible, strokeWidth, strokeOpacity = barStyle.opacity } = borderStyle; + const opacity = strokeOpacity * geometryStyle.opacity; - if (!visible || strokeWidth <= 0 || !stroke) { + if (!visible || strokeWidth <= 0 || !stroke || opacity <= 0) { return null; } - const opacity = geometryStyle.strokeOpacity; - return { x: x + strokeWidth / 2, y: y + strokeWidth / 2, diff --git a/src/utils/themes/theme.ts b/src/utils/themes/theme.ts index 58e284c820..08b440bfc8 100644 --- a/src/utils/themes/theme.ts +++ b/src/utils/themes/theme.ts @@ -22,7 +22,9 @@ export interface GeometryStyle { } export interface SharedGeometryStyle { - [key: string]: GeometryStyle; + default: GeometryStyle; + highlighted: GeometryStyle; + unhighlighted: GeometryStyle; } export interface StrokeStyle { From 679d30d8db8a39be7600cb36505ba5e004414c1e Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2019 21:19:48 -0500 Subject: [PATCH 3/5] test(rendering): update and add tests --- .../xy_chart/rendering/rendering.test.ts | 66 ++++++++----------- .../utils/rendering_props_utils.test.ts | 33 +++------- 2 files changed, 38 insertions(+), 61 deletions(-) diff --git a/src/chart_types/xy_chart/rendering/rendering.test.ts b/src/chart_types/xy_chart/rendering/rendering.test.ts index 0d1352809d..f392c0f3f9 100644 --- a/src/chart_types/xy_chart/rendering/rendering.test.ts +++ b/src/chart_types/xy_chart/rendering/rendering.test.ts @@ -1,4 +1,3 @@ -import { DEFAULT_GEOMETRY_STYLES } from '../../../utils/themes/theme_commons'; import { getSpecId } from '../../../utils/ids'; import { BarGeometry, @@ -8,7 +7,7 @@ import { getStyleOverrides, GeometryId, } from './rendering'; -import { BarSeriesStyle } from '../../../utils/themes/theme'; +import { BarSeriesStyle, SharedGeometryStyle } from '../../../utils/themes/theme'; import { DataSeriesDatum } from '../utils/series'; import { RecursivePartial, mergePartial } from '../../../utils/commons'; @@ -116,67 +115,58 @@ describe('Rendering utils', () => { }, }; - const sharedThemeStyle = DEFAULT_GEOMETRY_STYLES; - const specOpacity = 0.66; - - const defaultStyle = getGeometryStyle(geometryId, null, sharedThemeStyle); + const sharedThemeStyle: SharedGeometryStyle = { + default: { + opacity: 1, + }, + highlighted: { + opacity: 0.5, + }, + unhighlighted: { + opacity: 0.25, + }, + }; // no highlighted elements - expect(defaultStyle).toEqual({ opacity: 1 }); - - const customDefaultStyle = getGeometryStyle(geometryId, null, sharedThemeStyle, specOpacity); - - // no highlighted elements with custom spec opacity - expect(customDefaultStyle).toEqual({ opacity: 0.66 }); - - const highlightedStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedThemeStyle); + const defaultStyle = getGeometryStyle(geometryId, null, sharedThemeStyle); + expect(defaultStyle).toBe(sharedThemeStyle.default); // should equal highlighted opacity - expect(highlightedStyle).toEqual({ opacity: 1 }); - - const unhighlightedStyle = getGeometryStyle(geometryId, unhighlightedLegendItem, sharedThemeStyle); + const highlightedStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedThemeStyle); + expect(highlightedStyle).toBe(sharedThemeStyle.highlighted); // should equal unhighlighted opacity - expect(unhighlightedStyle).toEqual({ opacity: 0.25 }); - - const customHighlightedStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedThemeStyle, specOpacity); + const unhighlightedStyle = getGeometryStyle(geometryId, unhighlightedLegendItem, sharedThemeStyle); + expect(unhighlightedStyle).toBe(sharedThemeStyle.unhighlighted); // should equal custom spec highlighted opacity - expect(customHighlightedStyle).toEqual({ opacity: 0.66 }); - - const customUnhighlightedStyle = getGeometryStyle( - geometryId, - unhighlightedLegendItem, - sharedThemeStyle, - specOpacity, - ); + const customHighlightedStyle = getGeometryStyle(geometryId, highlightedLegendItem, sharedThemeStyle); + expect(customHighlightedStyle).toBe(sharedThemeStyle.highlighted); // unhighlighted elements remain unchanged with custom opacity - expect(customUnhighlightedStyle).toEqual({ opacity: 0.25 }); + const customUnhighlightedStyle = getGeometryStyle(geometryId, unhighlightedLegendItem, sharedThemeStyle); + expect(customUnhighlightedStyle).toBe(sharedThemeStyle.unhighlighted); // has individual highlight - const hasIndividualHighlight = getGeometryStyle(geometryId, null, sharedThemeStyle, undefined, { + const hasIndividualHighlight = getGeometryStyle(geometryId, null, sharedThemeStyle, { hasHighlight: true, hasGeometryHover: true, }); - - expect(hasIndividualHighlight).toEqual({ opacity: 1 }); + expect(hasIndividualHighlight).toBe(sharedThemeStyle.highlighted); // no highlight - const noHighlight = getGeometryStyle(geometryId, null, sharedThemeStyle, undefined, { + const noHighlight = getGeometryStyle(geometryId, null, sharedThemeStyle, { hasHighlight: false, hasGeometryHover: true, }); - - expect(noHighlight).toEqual({ opacity: 0.25 }); + expect(noHighlight).toBe(sharedThemeStyle.unhighlighted); // no geometry hover - const noHover = getGeometryStyle(geometryId, null, sharedThemeStyle, undefined, { + const noHover = getGeometryStyle(geometryId, null, sharedThemeStyle, { hasHighlight: true, hasGeometryHover: false, }); - - expect(noHover).toEqual({ opacity: 1 }); + expect(noHover).toBe(sharedThemeStyle.highlighted); }); describe('getStyleOverrides', () => { diff --git a/src/components/react_canvas/utils/rendering_props_utils.test.ts b/src/components/react_canvas/utils/rendering_props_utils.test.ts index 99e07a2ac2..e1b58a5968 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.test.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.test.ts @@ -35,7 +35,7 @@ describe('[canvas] Area Geometries props', () => { strokeEnabled: true, stroke: 'red', fill: 'red', - opacity: 0.2, + opacity: 0.2 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -64,7 +64,7 @@ describe('[canvas] Area Geometries props', () => { strokeEnabled: false, stroke: 'red', fill: 'blue', - opacity: 0.2, + opacity: 0.2 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -92,7 +92,7 @@ describe('[canvas] Area Geometries props', () => { strokeEnabled: true, stroke: 'violet', fill: 'pink', - opacity: 0.2, + opacity: 0.2 * 789, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -118,7 +118,7 @@ describe('[canvas] Area Geometries props', () => { fill: 'red', lineCap: 'round', lineJoin: 'round', - opacity: 0.8, + opacity: 0.8 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -144,7 +144,7 @@ describe('[canvas] Area Geometries props', () => { fill: 'blue', lineCap: 'round', lineJoin: 'round', - opacity: 1, + opacity: 1 * 123, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -202,7 +202,7 @@ describe('[canvas] Area Geometries props', () => { strokeWidth: 66, lineCap: 'round', lineJoin: 'round', - opacity: 0.5, + opacity: 0.5 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -234,7 +234,7 @@ describe('[canvas] Line Geometries', () => { strokeEnabled: true, stroke: 'pink', fill: 'pink', - opacity: 0.2, + opacity: 0.2 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -261,7 +261,7 @@ describe('[canvas] Line Geometries', () => { strokeEnabled: false, stroke: 'pink', fill: 'pink', - opacity: 0.2, + opacity: 0.2 * 0.5, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -289,7 +289,7 @@ describe('[canvas] Line Geometries', () => { strokeEnabled: true, stroke: 'series-stroke', fill: 'pink', - opacity: 0.2, + opacity: 0.2 * 18, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -366,11 +366,6 @@ describe('[canvas] Bar Geometries', () => { { opacity: 1, }, - { - stroke: 'blue', - strokeWidth: 1, - visible: true, - }, { opacity: 0.5, }, @@ -381,9 +376,7 @@ describe('[canvas] Bar Geometries', () => { width: 30, height: 40, fill: 'red', - stroke: 'blue', - strokeWidth: 1, - strokeEnabled: true, + strokeEnabled: false, strokeHitEnabled: false, perfectDrawEnabled: false, listening: false, @@ -399,10 +392,6 @@ describe('[canvas] Bar Geometries', () => { { opacity: 1, }, - { - strokeWidth: 0, - visible: true, - }, { opacity: 0.5, }, @@ -413,8 +402,6 @@ describe('[canvas] Bar Geometries', () => { width: 30, height: 40, fill: 'red', - stroke: 'transparent', - strokeWidth: 0, strokeEnabled: false, strokeHitEnabled: false, perfectDrawEnabled: false, From 9cdab92f1150e3da725e5bca54d1fc09b3ed378d Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Fri, 23 Aug 2019 11:41:59 -0500 Subject: [PATCH 4/5] test: add more tests --- .../utils/rendering_props_utils.test.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/components/react_canvas/utils/rendering_props_utils.test.ts b/src/components/react_canvas/utils/rendering_props_utils.test.ts index e1b58a5968..8d31b1d358 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.test.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.test.ts @@ -9,7 +9,9 @@ import { isBarValueOverflow, rotateBarValueProps, buildPointStyleProps, + buildBarBorderRenderProps, } from './rendering_props_utils'; +import { RectBorderStyle, RectStyle } from '../../../utils/themes/theme'; describe('[canvas] Area Geometries props', () => { test('can build area point props', () => { @@ -786,4 +788,79 @@ describe('[canvas] Bar Geometries', () => { expectedRotatedVerticalProps.y = 90; expect(rotatedVerticalContainedProps).toEqual(expectedRotatedVerticalProps); }); + + describe('buildBarBorderRenderProps', () => { + const getProps = (borderStyle: Partial = {}, barStyle: Partial = {}) => { + return [ + 10, + 20, + 30, + 40, + { + opacity: 1, + ...barStyle, + }, + { + strokeOpacity: 0.5, + strokeWidth: 2, + visible: true, + stroke: 'blue', + ...borderStyle, + }, + { + opacity: 0.5, + }, + ]; + }; + it('should build bar props with stroke', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps()); + expect(props).toEqual({ + x: 11, + y: 21, + width: 28, + height: 38, + fillEnabled: false, + strokeEnabled: true, + strokeWidth: 2, + stroke: 'blue', + strokeHitEnabled: false, + perfectDrawEnabled: false, + listening: false, + opacity: 0.5 * 0.5, + }); + }); + + it('should return null if visible is false', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps({ visible: false })); + expect(props).toBeNull(); + }); + + it('should return null if strokeWidth is 0 or less', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps({ strokeWidth: 0 })); + expect(props).toBeNull(); + }); + + it('should return null if no stroke color', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps({ stroke: undefined })); + expect(props).toBeNull(); + }); + + it('should return props with bar opacity if no stroke opacity', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps({ strokeOpacity: undefined })); + expect(props).toMatchObject({ + opacity: 1 * 0.5, + }); + }); + + it('should return null if no stroke opacity and bar opacity in 0', () => { + // @ts-ignore + const props = buildBarBorderRenderProps(...getProps({ strokeOpacity: undefined }, { opacity: 0 })); + expect(props).toBeNull(); + }); + }); }); From c7596a79040ffe6c51e926f444809dc1c7432745 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Fri, 23 Aug 2019 13:39:48 -0500 Subject: [PATCH 5/5] refactor: opacity border with inset rect --- .playground/playgroud.tsx | 6 +- .../react_canvas/bar_geometries.tsx | 12 +- .../utils/rendering_props_utils.test.ts | 416 +++++++++++------- .../utils/rendering_props_utils.ts | 15 +- 4 files changed, 288 insertions(+), 161 deletions(-) diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index 4719c9efd6..64064625b0 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -37,14 +37,14 @@ export class Playground extends React.Component { stackAccessors={[0]} barSeriesStyle={{ rectBorder: { - strokeOpacity: 0.5, + strokeOpacity: 1, strokeWidth: 4, - stroke: 'black', + stroke: 'blue', visible: true, }, rect: { opacity: 0.25, - fill: 'black', + fill: 'red', }, }} /> diff --git a/src/components/react_canvas/bar_geometries.tsx b/src/components/react_canvas/bar_geometries.tsx index 4e2f424a20..e10a09bfe0 100644 --- a/src/components/react_canvas/bar_geometries.tsx +++ b/src/components/react_canvas/bar_geometries.tsx @@ -80,6 +80,7 @@ export class BarGeometries extends React.PureComponent diff --git a/src/components/react_canvas/utils/rendering_props_utils.test.ts b/src/components/react_canvas/utils/rendering_props_utils.test.ts index 8d31b1d358..e5fe6e8a06 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.test.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.test.ts @@ -358,176 +358,290 @@ describe('[canvas] Line Geometries', () => { }); describe('[canvas] Bar Geometries', () => { - test('can build bar props', () => { - const props = buildBarRenderProps( - 10, - 20, - 30, - 40, - 'red', - { - opacity: 1, - }, - { + describe('buildBarValueProps', () => { + test('can build bar props', () => { + const props = buildBarRenderProps( + 10, + 20, + 30, + 40, + 'red', + { + opacity: 1, + }, + { + strokeOpacity: 0.5, + strokeWidth: 2, + visible: true, + stroke: 'blue', + }, + { + opacity: 0.5, + }, + ); + expect(props).toEqual({ + x: 12, + y: 22, + width: 26, + height: 36, + fill: 'red', + strokeEnabled: false, + strokeHitEnabled: false, + perfectDrawEnabled: false, + listening: false, opacity: 0.5, - }, - ); - expect(props).toEqual({ - x: 10, - y: 20, - width: 30, - height: 40, - fill: 'red', - strokeEnabled: false, - strokeHitEnabled: false, - perfectDrawEnabled: false, - listening: false, - opacity: 0.5, - }); + }); - const barWithNoBorder = buildBarRenderProps( - 10, - 20, - 30, - 40, - 'red', - { - opacity: 1, - }, - { + const barWithNoBorder = buildBarRenderProps( + 10, + 20, + 30, + 40, + 'red', + { + opacity: 1, + }, + { + strokeOpacity: 0.5, + strokeWidth: 2, + visible: true, + stroke: 'blue', + }, + { + opacity: 0.5, + }, + ); + expect(barWithNoBorder).toEqual({ + x: 12, + y: 22, + width: 26, + height: 36, + fill: 'red', + strokeEnabled: false, + strokeHitEnabled: false, + perfectDrawEnabled: false, + listening: false, opacity: 0.5, - }, - ); - expect(barWithNoBorder).toEqual({ - x: 10, - y: 20, - width: 30, - height: 40, - fill: 'red', - strokeEnabled: false, - strokeHitEnabled: false, - perfectDrawEnabled: false, - listening: false, - opacity: 0.5, + }); }); - }); - test('can build bar value props', () => { - const valueArguments = { - x: 10, - y: 20, - barWidth: 30, - barHeight: 40, - displayValueStyle: { - fill: 'fill', - fontFamily: 'ff', - fontSize: 10, - padding: 5, - offsetX: 0, - offsetY: 0, - }, - displayValue: { - text: 'foo', - width: 10, - height: 10, - isValueContainedInElement: false, - hideClippedValue: false, - }, - chartDimensions: { - width: 10, - height: 10, - top: 0, - left: 0, - }, - chartRotation: 0 as Rotation, + const getProps = (borderStyle: Partial = {}, barStyle: Partial = {}) => { + return [ + 10, + 20, + 30, + 40, + 'red', + { + opacity: 1, + ...barStyle, + }, + { + strokeOpacity: 0.5, + strokeWidth: 2, + visible: true, + stroke: 'blue', + ...borderStyle, + }, + { + opacity: 0.5, + }, + ]; }; - const basicProps = buildBarValueProps(valueArguments); - expect(basicProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 17.5, - y: 20, - height: 15, - width: 15, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should build bar props with stroke', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps()); + expect(props).toEqual({ + x: 12, + y: 22, + width: 26, + height: 36, + strokeEnabled: false, + strokeHitEnabled: false, + perfectDrawEnabled: false, + listening: false, + opacity: 0.5, + fill: 'red', + }); }); - valueArguments.barHeight = 2; - const insufficientHeightBarProps = buildBarValueProps(valueArguments); - expect(insufficientHeightBarProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 17.5, - y: 5, - height: 15, - width: 15, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should return dimensions without offset if visible is false', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps({ visible: false })); + expect(props).toMatchObject({ + x: 10, + y: 20, + width: 30, + height: 40, + }); }); - valueArguments.y = 4; - valueArguments.barHeight = 20; - valueArguments.chartDimensions = { - left: 0, - top: 0, - width: 0, - height: 0, - }; - const chartOverflowBarProps = buildBarValueProps(valueArguments); - expect(chartOverflowBarProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 17.5, - y: 4, - width: 15, - height: 15, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should return dimensions without offset if strokeWidth is 0 or less', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps({ strokeWidth: 0 })); + expect(props).toMatchObject({ + x: 10, + y: 20, + width: 30, + height: 40, + }); }); - valueArguments.displayValue.isValueContainedInElement = true; - const containedBarProps = buildBarValueProps(valueArguments); - expect(containedBarProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 17.5, - y: -21, - height: 25, - width: 15, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should return dimensions without offset if no stroke color', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps({ stroke: undefined })); + expect(props).toMatchObject({ + x: 10, + y: 20, + width: 30, + height: 40, + }); }); - valueArguments.displayValue.isValueContainedInElement = false; - valueArguments.barWidth = 0; - const containedXBarProps = buildBarValueProps(valueArguments); - expect(containedXBarProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 2.5, - y: 4, - height: 15, - width: 15, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should return dimensions without offset if no stroke opacity', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps({ strokeOpacity: undefined })); + expect(props).toMatchObject({ + x: 10, + y: 20, + width: 30, + height: 40, + opacity: 0.5, + }); }); - valueArguments.displayValue.hideClippedValue = true; - valueArguments.barWidth = 0; - const clippedBarProps = buildBarValueProps(valueArguments); - expect(clippedBarProps).toEqual({ - ...valueArguments.displayValueStyle, - x: 2.5, - y: 4, - height: 0, - width: 0, - align: 'center', - verticalAlign: 'top', - text: 'foo', + it('should return dimensions without offset if no stroke opacity and bar opacity in 0', () => { + // @ts-ignore + const props = buildBarRenderProps(...getProps({ strokeOpacity: undefined }, { opacity: 0 })); + expect(props).toMatchObject({ + x: 10, + y: 20, + width: 30, + height: 40, + }); + }); + }); + + describe('buildBarValueProps', () => { + test('can build bar value props', () => { + const valueArguments = { + x: 10, + y: 20, + barWidth: 30, + barHeight: 40, + displayValueStyle: { + fill: 'fill', + fontFamily: 'ff', + fontSize: 10, + padding: 5, + offsetX: 0, + offsetY: 0, + }, + displayValue: { + text: 'foo', + width: 10, + height: 10, + isValueContainedInElement: false, + hideClippedValue: false, + }, + chartDimensions: { + width: 10, + height: 10, + top: 0, + left: 0, + }, + chartRotation: 0 as Rotation, + }; + + const basicProps = buildBarValueProps(valueArguments); + expect(basicProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 17.5, + y: 20, + height: 15, + width: 15, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); + + valueArguments.barHeight = 2; + const insufficientHeightBarProps = buildBarValueProps(valueArguments); + expect(insufficientHeightBarProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 17.5, + y: 5, + height: 15, + width: 15, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); + + valueArguments.y = 4; + valueArguments.barHeight = 20; + valueArguments.chartDimensions = { + left: 0, + top: 0, + width: 0, + height: 0, + }; + const chartOverflowBarProps = buildBarValueProps(valueArguments); + expect(chartOverflowBarProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 17.5, + y: 4, + width: 15, + height: 15, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); + + valueArguments.displayValue.isValueContainedInElement = true; + const containedBarProps = buildBarValueProps(valueArguments); + expect(containedBarProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 17.5, + y: -21, + height: 25, + width: 15, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); + + valueArguments.displayValue.isValueContainedInElement = false; + valueArguments.barWidth = 0; + const containedXBarProps = buildBarValueProps(valueArguments); + expect(containedXBarProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 2.5, + y: 4, + height: 15, + width: 15, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); + + valueArguments.displayValue.hideClippedValue = true; + valueArguments.barWidth = 0; + const clippedBarProps = buildBarValueProps(valueArguments); + expect(clippedBarProps).toEqual({ + ...valueArguments.displayValueStyle, + x: 2.5, + y: 4, + height: 0, + width: 0, + align: 'center', + verticalAlign: 'top', + text: 'foo', + }); }); }); + test('shouold get clipDimensions for bar values', () => { const chartDimensions = { width: 100, diff --git a/src/components/react_canvas/utils/rendering_props_utils.ts b/src/components/react_canvas/utils/rendering_props_utils.ts index 9c7a472de3..0e78a0b35e 100644 --- a/src/components/react_canvas/utils/rendering_props_utils.ts +++ b/src/components/react_canvas/utils/rendering_props_utils.ts @@ -354,15 +354,18 @@ export function buildBarRenderProps( height: number, color: string, rectStyle: RectStyle, + borderStyle: RectBorderStyle, geometryStyle: GeometryStyle, ): RectConfig { const opacity = rectStyle.opacity * geometryStyle.opacity; + const { stroke, visible, strokeWidth, strokeOpacity = 0 } = borderStyle; + const offset = !visible || strokeWidth <= 0 || !stroke || strokeOpacity <= 0 || opacity <= 0 ? 0 : strokeWidth; return { - x, - y, - width, - height, + x: x + offset, + y: y + offset, + width: width - 2 * offset, + height: height - 2 * offset, fill: rectStyle.fill || color, strokeEnabled: false, ...geometryStyle, @@ -388,11 +391,11 @@ export function buildBarBorderRenderProps( y: number, width: number, height: number, - barStyle: RectStyle, + rectStyle: RectStyle, borderStyle: RectBorderStyle, geometryStyle: GeometryStyle, ): RectConfig | null { - const { stroke, visible, strokeWidth, strokeOpacity = barStyle.opacity } = borderStyle; + const { stroke, visible, strokeWidth, strokeOpacity = rectStyle.opacity } = borderStyle; const opacity = strokeOpacity * geometryStyle.opacity; if (!visible || strokeWidth <= 0 || !stroke || opacity <= 0) {