From ad43f1707856da7f67c4fc25a9b1fc0a78f5ac00 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Mon, 11 Jan 2021 11:26:54 -0800 Subject: [PATCH] new(xychart): add colorAccessor to relevant series (#1005) * new(xychart): add colorAccessor to BaseBarGroup/Stack/Series and BaseGlyphSeries * new(xychart/util): add cleanColorString + tests * fix(xychart): fix tweening of url-containing color strings * new(xychart): include index in colorAccessor signature * new(demo/xychart): demo colorAccessor * test(xychart): add colorAccessor tests * style(demo/xychart): improve visibility of glyph patterns * type(xychart): fix colorAccessor generics in BarSeries --- .../src/sandboxes/visx-xychart/Example.tsx | 6 +++ .../visx-xychart/ExampleControls.tsx | 39 +++++++++++++++---- .../components/series/AnimatedBarSeries.tsx | 12 +++++- .../src/components/series/BarSeries.tsx | 17 ++++++-- .../series/private/AnimatedBars.tsx | 8 ++-- .../series/private/AnimatedGlyphs.tsx | 13 ++++--- .../series/private/BaseBarGroup.tsx | 4 +- .../series/private/BaseBarSeries.tsx | 17 +++++++- .../series/private/BaseBarStack.tsx | 13 ++++++- .../series/private/BaseGlyphSeries.tsx | 8 ++-- .../src/utils/cleanColorString.ts | 4 ++ .../test/components/BarGroup.test.tsx | 22 +++++++++++ .../test/components/BarSeries.test.tsx | 17 ++++++++ .../test/components/BarStack.test.tsx | 22 +++++++++++ .../test/components/GlyphSeries.test.tsx | 17 ++++++++ .../test/utils/cleanColorString.test.ts | 15 +++++++ 16 files changed, 206 insertions(+), 28 deletions(-) create mode 100644 packages/visx-xychart/src/utils/cleanColorString.ts create mode 100644 packages/visx-xychart/test/utils/cleanColorString.test.ts diff --git a/packages/visx-demo/src/sandboxes/visx-xychart/Example.tsx b/packages/visx-demo/src/sandboxes/visx-xychart/Example.tsx index bd3c2412d..2589685ac 100644 --- a/packages/visx-demo/src/sandboxes/visx-xychart/Example.tsx +++ b/packages/visx-demo/src/sandboxes/visx-xychart/Example.tsx @@ -37,6 +37,7 @@ export default function Example({ height }: XYChartProps) { annotationDatum, annotationLabelPosition, annotationType, + colorAccessorFactory, config, curve, data, @@ -113,18 +114,21 @@ export default function Example({ height }: XYChartProps) { data={data} xAccessor={accessors.x['New York']} yAccessor={accessors.y['New York']} + colorAccessor={colorAccessorFactory('New York')} /> )} @@ -134,6 +138,7 @@ export default function Example({ height }: XYChartProps) { data={data} xAccessor={accessors.x['New York']} yAccessor={accessors.y['New York']} + colorAccessor={colorAccessorFactory('New York')} /> )} {renderAreaSeries && ( @@ -200,6 +205,7 @@ export default function Example({ height }: XYChartProps) { xAccessor={accessors.x['San Francisco']} yAccessor={accessors.y['San Francisco']} renderGlyph={renderGlyph} + colorAccessor={colorAccessorFactory('San Francisco')} /> )} -getSfTemperature(d); const getNyTemperature = (d: CityTemperature) => Number(d['New York']); const getAustinTemperature = (d: CityTemperature) => Number(d.Austin); const defaultAnnotationDataIndex = 13; +const selectedDatumPatternId = 'xychart-selected-datum'; type Accessor = (d: CityTemperature) => number | string; @@ -35,6 +37,8 @@ interface Accessors { Austin: Accessor; } +type DataKey = keyof Accessors; + type SimpleScaleConfig = { type: 'band' | 'linear'; paddingInner?: number }; type ProvidedProps = { @@ -44,10 +48,11 @@ type ProvidedProps = { date: Accessor; }; animationTrajectory: AnimationTrajectory; - annotationDataKey: keyof Accessors | null; + annotationDataKey: DataKey | null; annotationDatum?: CityTemperature; annotationLabelPosition: { dx: number; dy: number }; annotationType?: 'line' | 'circle'; + colorAccessorFactory: (key: DataKey) => (d: CityTemperature) => string | null; config: { x: SimpleScaleConfig; y: SimpleScaleConfig; @@ -57,7 +62,7 @@ type ProvidedProps = { editAnnotationLabelPosition: boolean; numTicks: number; setAnnotationDataIndex: (index: number) => void; - setAnnotationDataKey: (key: keyof Accessors | null) => void; + setAnnotationDataKey: (key: DataKey | null) => void; setAnnotationLabelPosition: (position: { dx: number; dy: number }) => void; renderAreaSeries: boolean; renderBarGroup: boolean; @@ -117,18 +122,18 @@ export default function ExampleControls({ children }: ControlsProps) { const [missingValues, setMissingValues] = useState(false); const [glyphComponent, setGlyphComponent] = useState<'star' | 'cross' | 'circle' | '🍍'>('star'); const [curveType, setCurveType] = useState<'linear' | 'cardinal' | 'step'>('linear'); - const themeBackground = theme.backgroundColor; + const glyphOutline = theme.gridStyles.stroke; const renderGlyph = useCallback( ({ size, color, onPointerMove, onPointerOut, onPointerUp }: GlyphProps) => { const handlers = { onPointerMove, onPointerOut, onPointerUp }; if (glyphComponent === 'star') { - return ; + return ; } if (glyphComponent === 'circle') { - return ; + return ; } if (glyphComponent === 'cross') { - return ; + return ; } return ( @@ -136,7 +141,15 @@ export default function ExampleControls({ children }: ControlsProps) { ); }, - [glyphComponent, themeBackground], + [glyphComponent, glyphOutline], + ); + // for series that support it, return a colorAccessor which returns a custom color if the datum is selected + const colorAccessorFactory = useCallback( + (dataKey: DataKey) => (d: CityTemperature) => + annotationDataKey === dataKey && d === data[annotationDataIndex] + ? `url(#${selectedDatumPatternId})` + : null, + [annotationDataIndex, annotationDataKey], ); const accessors = useMemo( @@ -183,6 +196,7 @@ export default function ExampleControls({ children }: ControlsProps) { annotationDatum: data[annotationDataIndex], annotationLabelPosition, annotationType, + colorAccessorFactory, config, curve: (curveType === 'cardinal' && curveCardinal) || @@ -220,6 +234,17 @@ export default function ExampleControls({ children }: ControlsProps) { xAxisOrientation, yAxisOrientation, })} + {/** This style is used for annotated elements via colorAccessor. */} + + +
{/** data */}
diff --git a/packages/visx-xychart/src/components/series/AnimatedBarSeries.tsx b/packages/visx-xychart/src/components/series/AnimatedBarSeries.tsx index 8f7efdc64..0ae2b017f 100644 --- a/packages/visx-xychart/src/components/series/AnimatedBarSeries.tsx +++ b/packages/visx-xychart/src/components/series/AnimatedBarSeries.tsx @@ -7,6 +7,14 @@ export default function AnimatedBarSeries< XScale extends AxisScale, YScale extends AxisScale, Datum extends object ->({ ...props }: Omit, 'BarsComponent'>) { - return {...props} BarsComponent={AnimatedBars} />; +>({ colorAccessor, ...props }: Omit, 'BarsComponent'>) { + return ( + + {...props} + // @TODO currently generics for non-SeriesProps are not passed correctly in + // withRegisteredData HOC + colorAccessor={colorAccessor as BaseBarSeriesProps['colorAccessor']} + BarsComponent={AnimatedBars} + /> + ); } diff --git a/packages/visx-xychart/src/components/series/BarSeries.tsx b/packages/visx-xychart/src/components/series/BarSeries.tsx index 3e6d08a22..6768ef17a 100644 --- a/packages/visx-xychart/src/components/series/BarSeries.tsx +++ b/packages/visx-xychart/src/components/series/BarSeries.tsx @@ -3,10 +3,19 @@ import React from 'react'; import BaseBarSeries, { BaseBarSeriesProps } from './private/BaseBarSeries'; import Bars from './private/Bars'; -function BarSeries( - props: Omit, 'BarsComponent'>, -) { - return {...props} BarsComponent={Bars} />; +function BarSeries({ + colorAccessor, + ...props +}: Omit, 'BarsComponent'>) { + return ( + + {...props} + // @TODO currently generics for non-SeriesProps are not passed correctly in + // withRegisteredData HOC + colorAccessor={colorAccessor as BaseBarSeriesProps['colorAccessor']} + BarsComponent={Bars} + /> + ); } export default BarSeries; diff --git a/packages/visx-xychart/src/components/series/private/AnimatedBars.tsx b/packages/visx-xychart/src/components/series/private/AnimatedBars.tsx index 8793db5cf..ec9347dc4 100644 --- a/packages/visx-xychart/src/components/series/private/AnimatedBars.tsx +++ b/packages/visx-xychart/src/components/series/private/AnimatedBars.tsx @@ -2,6 +2,7 @@ import { AxisScale } from '@visx/axis'; import React, { useMemo } from 'react'; import { animated, useTransition } from 'react-spring'; import { Bar, BarsProps } from '../../../types'; +import { cleanColor, colorHasUrl } from '../../../utils/cleanColorString'; import getScaleBaseline from '../../../utils/getScaleBaseline'; function enterUpdate({ x, y, width, height, fill }: Bar) { @@ -10,7 +11,7 @@ function enterUpdate({ x, y, width, height, fill }: Bar) { y, width, height, - fill, + fill: cleanColor(fill), opacity: 1, }; } @@ -34,7 +35,7 @@ function useBarTransitionConfig({ y: shouldAnimateX ? y : scaleBaseline ?? 0, width: shouldAnimateX ? 0 : width, height: shouldAnimateX ? height : 0, - fill, + fill: cleanColor(fill), opacity: 0, }; } @@ -72,7 +73,8 @@ export default function AnimatedBars diff --git a/packages/visx-xychart/src/components/series/private/AnimatedGlyphs.tsx b/packages/visx-xychart/src/components/series/private/AnimatedGlyphs.tsx index 89c2d8c3d..98cac4cc8 100644 --- a/packages/visx-xychart/src/components/series/private/AnimatedGlyphs.tsx +++ b/packages/visx-xychart/src/components/series/private/AnimatedGlyphs.tsx @@ -3,6 +3,7 @@ import React, { useMemo } from 'react'; import { useTransition, animated, interpolate } from 'react-spring'; import getScaleBaseline from '../../../utils/getScaleBaseline'; import { GlyphProps, GlyphsProps } from '../../../types'; +import { cleanColor, colorHasUrl } from '../../../utils/cleanColorString'; type ConfigKeys = 'enter' | 'update' | 'from' | 'leave'; @@ -30,17 +31,17 @@ export function useAnimatedGlyphsConfig< from: ({ x, y, color }) => ({ x: horizontal ? xScaleBaseline : x, y: horizontal ? y : yScaleBaseline, - color, + color: cleanColor(color), opacity: 0, }), leave: ({ x, y, color }) => ({ x: horizontal ? xScaleBaseline : x, y: horizontal ? y : yScaleBaseline, - color, + color: cleanColor(color), opacity: 0, }), - enter: ({ x, y, color }) => ({ x, y, color, opacity: 1 }), - update: ({ x, y, color }) => ({ x, y, color, opacity: 1 }), + enter: ({ x, y, color }) => ({ x, y, color: cleanColor(color), opacity: 1 }), + update: ({ x, y, color }) => ({ x, y, color: cleanColor(color), opacity: 1 }), }), [xScaleBaseline, yScaleBaseline, horizontal], ); @@ -91,7 +92,9 @@ export default function AnimatedGlyphs< x: 0, y: 0, size: item.size, - color: 'currentColor', // allows us to animate the color of the element + // currentColor doesn't work with url-based colors (pattern, gradient) + // otherwise currentColor allows us to animate the color of the element + color: colorHasUrl(item.color) ? item.color : 'currentColor', onBlur, onFocus, onPointerMove, diff --git a/packages/visx-xychart/src/components/series/private/BaseBarGroup.tsx b/packages/visx-xychart/src/components/series/private/BaseBarGroup.tsx index 1f959920e..ad98cfabb 100644 --- a/packages/visx-xychart/src/components/series/private/BaseBarGroup.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseBarGroup.tsx @@ -153,6 +153,8 @@ export default function BaseBarGroup< const getWidth = horizontal ? (d: Datum) => Math.abs(getLength(d)) : () => barThickness; const getHeight = horizontal ? () => barThickness : (d: Datum) => Math.abs(getLength(d)); + const colorAccessor = barSeriesChildren.find(child => child.props.dataKey === key)?.props + ?.colorAccessor; return data .map((bar, index) => { @@ -171,7 +173,7 @@ export default function BaseBarGroup< y: barY, width: barWidth, height: barHeight, - fill: colorScale(key), + fill: colorAccessor?.(bar, index) ?? colorScale(key), }; }) .filter(bar => bar) as Bar[]; diff --git a/packages/visx-xychart/src/components/series/private/BaseBarSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseBarSeries.tsx index 244c378e7..3f9640c54 100644 --- a/packages/visx-xychart/src/components/series/private/BaseBarSeries.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseBarSeries.tsx @@ -22,6 +22,8 @@ export type BaseBarSeriesProps< * Accepted values are [0, 1], 0 = no padding, 1 = no bar, defaults to 0.1. */ barPadding?: number; + /** Given a Datum, returns its color. Falls back to theme color if unspecified or if a null-ish value is returned. */ + colorAccessor?: (d: Datum, index: number) => string | null | undefined; }; // Fallback bandwidth estimate assumes no missing data values (divides chart space by # datum) @@ -32,6 +34,7 @@ const getFallbackBandwidth = (fullBarWidth: number, barPadding: number) => function BaseBarSeries({ BarsComponent, barPadding = 0.1, + colorAccessor, data, dataKey, onBlur, @@ -78,11 +81,21 @@ function BaseBarSeries bar) as Bar[]; - }, [barThickness, color, data, getScaledX, getScaledY, horizontal, xZeroPosition, yZeroPosition]); + }, [ + barThickness, + color, + colorAccessor, + data, + getScaledX, + getScaledY, + horizontal, + xZeroPosition, + yZeroPosition, + ]); const ownEventSourceKey = `${BARSERIES_EVENT_SOURCE}-${dataKey}`; const eventEmitters = useSeriesEvents({ diff --git a/packages/visx-xychart/src/components/series/private/BaseBarStack.tsx b/packages/visx-xychart/src/components/series/private/BaseBarStack.tsx index f4f41cafc..851f885bd 100644 --- a/packages/visx-xychart/src/components/series/private/BaseBarStack.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseBarStack.tsx @@ -197,6 +197,12 @@ function BaseBarStack< const entry = dataRegistry.get(barStack.key); if (!entry) return null; + // get colorAccessor from child BarSeries, if available + const barSeries: + | React.ReactElement> + | undefined = barSeriesChildren.find(child => child.props.dataKey === barStack.key); + const colorAccessor = barSeries?.props?.colorAccessor; + return barStack.map((bar, index) => { const barX = getX(bar); if (!isValidNumber(barX)) return null; @@ -207,13 +213,18 @@ function BaseBarStack< const barHeight = getHeight(bar); if (!isValidNumber(barHeight)) return null; + const barSeriesDatum = colorAccessor ? barSeries?.props?.data[index] : null; + return { key: `${stackIndex}-${barStack.key}-${index}`, x: barX, y: barY, width: barWidth, height: barHeight, - fill: colorScale(barStack.key), + fill: + barSeriesDatum && colorAccessor + ? colorAccessor(barSeriesDatum, index) + : colorScale(barStack.key), }; }); }) diff --git a/packages/visx-xychart/src/components/series/private/BaseGlyphSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseGlyphSeries.tsx index 707563066..3d1925ef3 100644 --- a/packages/visx-xychart/src/components/series/private/BaseGlyphSeries.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseGlyphSeries.tsx @@ -13,6 +13,8 @@ export type BaseGlyphSeriesProps< YScale extends AxisScale, Datum extends object > = SeriesProps & { + /** Given a Datum, returns its color. Falls back to theme color if unspecified or if a null-ish value is returned. */ + colorAccessor?: (d: Datum, index: number) => string | null | undefined; /** The size of a `Glyph`, a `number` or a function which takes a `Datum` and returns a `number`. */ size?: number | ((d: Datum) => number); /** Function which handles rendering glyphs. */ @@ -24,6 +26,7 @@ export function BaseGlyphSeries< YScale extends AxisScale, Datum extends object >({ + colorAccessor, data, dataKey, onBlur, @@ -42,7 +45,6 @@ export function BaseGlyphSeries< const { colorScale, theme, horizontal } = useContext(DataContext); const getScaledX = useCallback(getScaledValueFactory(xScale, xAccessor), [xScale, xAccessor]); const getScaledY = useCallback(getScaledValueFactory(yScale, yAccessor), [yScale, yAccessor]); - // @TODO allow override const color = colorScale?.(dataKey) ?? theme?.colors?.[0] ?? '#222'; const ownEventSourceKey = `${GLYPHSERIES_EVENT_SOURCE}-${dataKey}`; @@ -70,13 +72,13 @@ export function BaseGlyphSeries< key: `${i}`, x, y, - color, + color: colorAccessor?.(datum, i) ?? color, size: typeof size === 'function' ? size(datum) : size, datum, }; }) .filter(point => point) as GlyphProps[], - [getScaledX, getScaledY, data, size, color], + [color, colorAccessor, data, getScaledX, getScaledY, size], ); return ( diff --git a/packages/visx-xychart/src/utils/cleanColorString.ts b/packages/visx-xychart/src/utils/cleanColorString.ts new file mode 100644 index 000000000..a9c5281d0 --- /dev/null +++ b/packages/visx-xychart/src/utils/cleanColorString.ts @@ -0,0 +1,4 @@ +/* react-spring cannot tween colors with url ids (patterns, gradients), these helpers detect and clean them. */ +const neutralCleanColor = 'rgba(0,0,0,0.1)'; +export const colorHasUrl = (color?: string) => Boolean(color?.includes('url(')); +export const cleanColor = (color?: string) => (colorHasUrl(color) ? neutralCleanColor : color); diff --git a/packages/visx-xychart/test/components/BarGroup.test.tsx b/packages/visx-xychart/test/components/BarGroup.test.tsx index 64f53e19b..f8e0785a4 100644 --- a/packages/visx-xychart/test/components/BarGroup.test.tsx +++ b/packages/visx-xychart/test/components/BarGroup.test.tsx @@ -59,6 +59,28 @@ describe('', () => { expect(wrapper.find('rect')).toHaveLength(4); }); + it('should use colorAccessor if passed', () => { + const wrapper = mount( + + + + + (i === 0 ? 'banana' : null)} + /> + + + , + ); + const rects = wrapper.find('rect'); + expect(rects.at(0).prop('fill')).not.toBe('banana'); + expect(rects.at(1).prop('fill')).not.toBe('banana'); + expect(rects.at(2).prop('fill')).toBe('banana'); + expect(rects.at(3).prop('fill')).not.toBe('banana'); + }); + it('should not render rects with invalid x or y', () => { const wrapper = mount( diff --git a/packages/visx-xychart/test/components/BarSeries.test.tsx b/packages/visx-xychart/test/components/BarSeries.test.tsx index ba06b50e7..35707adb0 100644 --- a/packages/visx-xychart/test/components/BarSeries.test.tsx +++ b/packages/visx-xychart/test/components/BarSeries.test.tsx @@ -30,6 +30,23 @@ describe('', () => { expect(wrapper.find('rect')).toHaveLength(2); }); + it('should use colorAccessor if passed', () => { + const wrapper = mount( + + + (i === 0 ? 'banana' : null)} + /> + + , + ); + const rects = wrapper.find('rect'); + expect(rects.at(0).prop('fill')).toBe('banana'); + expect(rects.at(1).prop('fill')).not.toBe('banana'); + }); + it('should not render rects if x or y is invalid', () => { const wrapper = mount( diff --git a/packages/visx-xychart/test/components/BarStack.test.tsx b/packages/visx-xychart/test/components/BarStack.test.tsx index 431847156..af988f6a2 100644 --- a/packages/visx-xychart/test/components/BarStack.test.tsx +++ b/packages/visx-xychart/test/components/BarStack.test.tsx @@ -65,6 +65,28 @@ describe('', () => { expect(wrapper.find('rect')).toHaveLength(4); }); + it('should use colorAccessor if passed', () => { + const wrapper = mount( + + + + + (i === 0 ? 'banana' : null)} + /> + + + , + ); + const rects = wrapper.find('rect'); + expect(rects.at(0).prop('fill')).not.toBe('banana'); + expect(rects.at(1).prop('fill')).not.toBe('banana'); + expect(rects.at(2).prop('fill')).toBe('banana'); + expect(rects.at(3).prop('fill')).not.toBe('banana'); + }); + it('should not render rects if x or y are invalid', () => { const wrapper = mount( diff --git a/packages/visx-xychart/test/components/GlyphSeries.test.tsx b/packages/visx-xychart/test/components/GlyphSeries.test.tsx index 90b0191e4..009bad7ad 100644 --- a/packages/visx-xychart/test/components/GlyphSeries.test.tsx +++ b/packages/visx-xychart/test/components/GlyphSeries.test.tsx @@ -30,6 +30,23 @@ describe('', () => { expect(wrapper.find('circle')).toHaveLength(series.data.length); }); + it('should use colorAccessor if passed', () => { + const wrapper = mount( + + + (i === 0 ? 'banana' : null)} + /> + + , + ); + const circles = wrapper.find('circle'); + expect(circles.at(0).prop('fill')).toBe('banana'); + expect(circles.at(1).prop('fill')).not.toBe('banana'); + }); + it('should not render Glyphs if x or y is invalid', () => { const wrapper = mount( diff --git a/packages/visx-xychart/test/utils/cleanColorString.test.ts b/packages/visx-xychart/test/utils/cleanColorString.test.ts new file mode 100644 index 000000000..0e93639c0 --- /dev/null +++ b/packages/visx-xychart/test/utils/cleanColorString.test.ts @@ -0,0 +1,15 @@ +import { cleanColor } from '../../src/utils/cleanColorString'; + +describe('cleanColorString', () => { + describe('cleanColor', () => { + it('should be defined', () => { + expect(cleanColor).toBeDefined(); + }); + it('should return input color for non-url colors', () => { + expect(cleanColor('violet')).toBe('violet'); + }); + it('should return a neutral color for url-containing colors', () => { + expect(cleanColor('url(#id)')).not.toBe('url(#id)'); + }); + }); +});