From 3197be5daf9ee05350826e8d1a4d85d397ebf745 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 2 Jul 2020 18:42:08 -0700 Subject: [PATCH] new(vx-chart-poc/withRegisteredData): ensure props.data === dataRegistry.data, misc fixes --- .../src/components/series/AnimatedBars.tsx | 14 +++-- .../src/components/series/BarSeries.tsx | 58 ++++++++++--------- .../src/components/series/Group.tsx | 27 +++++---- .../src/components/series/Stack.tsx | 21 +++++-- .../src/enhancers/withRegisteredData.tsx | 6 +- 5 files changed, 76 insertions(+), 50 deletions(-) diff --git a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/AnimatedBars.tsx b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/AnimatedBars.tsx index 0f7672ab9..6fc9f032f 100644 --- a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/AnimatedBars.tsx +++ b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/AnimatedBars.tsx @@ -10,10 +10,16 @@ export type AnimatedBarsProps = { y?: DimensionAccessor; width?: DimensionAccessor; height?: DimensionAccessor; - rx?: number; -}; +} & Omit, 'x' | 'y' | 'width' | 'height' | 'ref'>; -export default function AnimatedBars({ bars, x, y, width, height, rx }: AnimatedBarsProps) { +export default function AnimatedBars({ + bars, + x, + y, + width, + height, + ...rectProps +}: AnimatedBarsProps) { const animatedBars = useSprings( bars.length, bars.map(bar => ({ @@ -37,7 +43,7 @@ export default function AnimatedBars({ bars, x, y, width, height, rx }: Animated width={bar.width} height={bar.height} fill={bar.color} - rx={rx} + {...rectProps} /> ))} diff --git a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/BarSeries.tsx b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/BarSeries.tsx index e149cb0c5..f7f249219 100644 --- a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/BarSeries.tsx +++ b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/BarSeries.tsx @@ -1,5 +1,4 @@ -import React, { useContext, useCallback } from 'react'; -import { animated, useSprings } from 'react-spring'; +import React, { useContext, useCallback, useMemo } from 'react'; import ChartContext from '../../context/ChartContext'; import { ChartContext as ChartContextType, SeriesProps } from '../../types'; import withRegisteredData from '../../enhancers/withRegisteredData'; @@ -7,6 +6,7 @@ import isValidNumber from '../../typeguards/isValidNumber'; import useRegisteredData from '../../hooks/useRegisteredData'; import findNearestDatumX from '../../util/findNearestDatumX'; import findNearestDatumY from '../../util/findNearestDatumY'; +import AnimatedBars from './AnimatedBars'; type BarSeriesProps = SeriesProps< Datum, @@ -67,36 +67,38 @@ function BarSeries { - const x = getScaledX(datum); - const y = getScaledY(datum); - const barLength = horizontal ? x - xZeroPosition : y - yZeroPosition; + const barColor = colorScale(dataKey) as string; - return { - x: horizontal ? xZeroPosition + Math.min(0, barLength) : x, - y: horizontal ? y : yZeroPosition + Math.min(0, barLength), - width: horizontal ? Math.abs(barLength) : barThickness, - height: horizontal ? barThickness : Math.abs(barLength), - }; - }), - ) as { x: number; y: number; width: number; height: number }[]; + const bars = useMemo( + () => + data.map(datum => { + const x = getScaledX(datum); + const y = getScaledY(datum); + const barLength = horizontal ? x - xZeroPosition : y - yZeroPosition; + + return { + x: horizontal ? xZeroPosition + Math.min(0, barLength) : x, + y: horizontal ? y : yZeroPosition + Math.min(0, barLength), + width: horizontal ? Math.abs(barLength) : barThickness, + height: horizontal ? barThickness : Math.abs(barLength), + color: barColor, + }; + }), + [ + horizontal, + barColor, + barThickness, + data, + xZeroPosition, + yZeroPosition, + getScaledX, + getScaledY, + ], + ); return ( - {animatedBars.map(({ x, y, width, height }, i) => ( - - ))} + ); } diff --git a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Group.tsx b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Group.tsx index 393011aa1..9d851b366 100644 --- a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Group.tsx +++ b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Group.tsx @@ -16,12 +16,13 @@ const GROUP_ACCESSOR = d => d.group; export type GroupProps = { horizontal?: boolean; children: typeof BarSeries; -}; +} & Omit, 'x' | 'y' | 'width' | 'height' | 'ref'>; // @TODO add GroupKeys type export default function Group({ horizontal, children, + ...rectProps }: GroupProps) { const { width, @@ -41,7 +42,8 @@ export default function Group({ [children], ); - const withinGroupScale = useMemo( + // + const groupScale = useMemo( () => scaleBand({ domain: [...dataKeys], @@ -52,7 +54,7 @@ export default function Group({ ); // @todo, this should be refactored such that it can be memoized. - // currently it references withinGroupScale which depends on xScale, yScale, + // currently it references groupScale which depends on xScale, yScale, // and thus causes an infinite loop for updating the data registry. const findNearestDatum = (args: NearestDatumArgs) => { const nearestDatum = horizontal @@ -66,16 +68,16 @@ export default function Group({ : Math.abs( args.svgMouseX - (args.xScale(args.xAccessor(nearestDatum.datum)) + - withinGroupScale(args.key) + - withinGroupScale.bandwidth() / 2), + groupScale(args.key) + + groupScale.bandwidth() / 2), ); const distanceY = horizontal ? Math.abs( args.svgMouseY - (args.yScale(args.yAccessor(nearestDatum.datum)) + - withinGroupScale(args.key) + - withinGroupScale.bandwidth() / 2), + groupScale(args.key) + + groupScale.bandwidth() / 2), ) : nearestDatum.distanceY; @@ -100,7 +102,7 @@ export default function Group({ return () => unregisterData(Object.keys(dataToRegister)); }, // @TODO fix findNearestDatum - // can't include findNearestDatum as it depends on withinGroupScale which depends + // can't include findNearestDatum as it depends on groupScale which depends // on the registry so will cause an infinite loop. [registerData, unregisterData, children], ); @@ -132,9 +134,9 @@ export default function Group({ } // @TODO handle NaNs from non-number inputs, prob fallback to 0 + // @TODO should consider refactoring base shapes to handle negative values better const scaledZeroPosition = (horizontal ? xScale : yScale)(0); - // @TODO should consider refactoring base shapes to handle negative values better return horizontal ? ( data={combinedData} @@ -143,12 +145,13 @@ export default function Group({ x={xValue => xScale(xValue)} y0={GROUP_ACCESSOR} y0Scale={yScale} // group position - y1Scale={withinGroupScale} + y1Scale={groupScale} xScale={xScale} color={colorScale} > {barGroups => barGroups.map(barGroup => ( + // @TODO if we use we might be able to make this animate on first render ({ width={bar => Math.abs(bar.width - scaledZeroPosition)} height={bar => bar.height} rx={2} + {...rectProps} /> )) @@ -169,7 +173,7 @@ export default function Group({ height={height - margin.top - margin.bottom} // BarGroup should figure this out from yScale x0={GROUP_ACCESSOR} x0Scale={xScale} // group position - x1Scale={withinGroupScale} + x1Scale={groupScale} yScale={yScale} color={dataKey => colorScale(dataKey) as string} > @@ -183,6 +187,7 @@ export default function Group({ width={bar => bar.width} height={bar => Math.abs(scaledZeroPosition - bar.y)} rx={2} + {...rectProps} /> )) diff --git a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Stack.tsx b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Stack.tsx index 92600bf3a..3b4f51e26 100644 --- a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Stack.tsx +++ b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/components/series/Stack.tsx @@ -16,14 +16,13 @@ const STACK_ACCESSOR = d => d.stack; export type GroupProps = { horizontal?: boolean; children: typeof BarSeries; -}; +} & Omit, 'x' | 'y' | 'width' | 'height' | 'ref'>; export default function Stack({ horizontal, children, + ...rectProps }: GroupProps) { - console.log('render Stack'); - const { xScale, yScale, colorScale, dataRegistry, registerData, unregisterData, height, margin } = (useContext(ChartContext) as ChartContextType) || {}; @@ -36,6 +35,7 @@ export default function Stack({ // use a ref to the stacks for mouse movements const stacks = useRef[] | null>(null); + // override the findNearestDatum logic const findNearestDatum = useCallback( (args: NearestDatumArgs) => { if (!stacks.current) return; @@ -107,6 +107,7 @@ export default function Stack({ return Object.values(dataByStackValue); }, [horizontal, children]); + // update the domain to account for the (directional) stacked value const comprehensiveDomain: number[] = useMemo( () => extent( @@ -132,6 +133,8 @@ export default function Stack({ }); registerData(dataToRegister); + + // unregister data on unmount return () => unregisterData(Object.keys(dataToRegister)); }, [ horizontal, @@ -166,7 +169,11 @@ export default function Stack({ // use this reference to find nearest mouse values stacks.current = barStacks; return barStacks.map((barStack, index) => ( - + )); }} @@ -186,7 +193,11 @@ export default function Stack({ // use this reference to find nearest mouse values stacks.current = barStacks; return barStacks.map((barStack, index) => ( - + )); }} diff --git a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/enhancers/withRegisteredData.tsx b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/enhancers/withRegisteredData.tsx index 8ba006ded..9011f29c6 100644 --- a/packages/vx-demo/src/sandboxes/vx-chart-poc/src/enhancers/withRegisteredData.tsx +++ b/packages/vx-demo/src/sandboxes/vx-chart-poc/src/enhancers/withRegisteredData.tsx @@ -27,7 +27,7 @@ export default function withRegisteredData< ) { const WrappedSeriesComponent: FunctionComponent = props => { const { dataKey, data, xAccessor, yAccessor, mouseEvents } = props; - const { xScale, yScale } = useContext(ChartContext); + const { xScale, yScale, dataRegistry } = useContext(ChartContext); useDataRegistry({ key: dataKey, @@ -39,7 +39,9 @@ export default function withRegisteredData< findNearestDatum: findNearestDatum?.(props), }); - return xScale && yScale ? : null; + return xScale && yScale && dataRegistry?.[dataKey]?.data === data ? ( + + ) : null; }; return WrappedSeriesComponent;