new(vx-chart-poc/withRegisteredData): ensure props.data === dataRegis…

…try.data, misc fixes
airbnb · Jul 3, 2020 · 3197be5 · 3197be5
1 parent ad9f538
commit 3197be5
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 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
@@ -10,10 +10,16 @@ export type AnimatedBarsProps = {
   y?: DimensionAccessor;
   width?: DimensionAccessor;
   height?: DimensionAccessor;
-  rx?: number;
-};
+} & Omit<React.SVGProps<SVGRectElement>, '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
@@ -1,12 +1,12 @@
-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';
 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<Datum, XScaleInput, YScaleInput> = SeriesProps<
   Datum,
@@ -67,36 +67,38 @@ function BarSeries<Datum = unknown, XScaleInput = unknown, YScaleInput = unknown
     ? (Math.min(maybeYZero, Math.max(yMin, yMax)) as number)
     : Math.max(yMin, yMax);
 
-  const animatedBars = useSprings(
-    data.length,
-    data.map(datum => {
-      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 (
     <g className="vx-chart bar-series">
-      {animatedBars.map(({ x, y, width, height }, i) => (
-        <animated.rect
-          key={i}
-          x={x}
-          y={y}
-          width={width}
-          height={height}
-          fill={colorScale(dataKey)}
-          stroke={theme.baseColor ?? 'white'}
-          {...barProps}
-        />
-      ))}
+      <AnimatedBars bars={bars} stroke={theme.baseColor ?? 'white'} {...barProps} />
     </g>
   );
 }

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
@@ -16,12 +16,13 @@ const GROUP_ACCESSOR = d => d.group;
 export type GroupProps = {
   horizontal?: boolean;
   children: typeof BarSeries;
-};
+} & Omit<React.SVGProps<SVGRectElement>, 'x' | 'y' | 'width' | 'height' | 'ref'>;
 
 // @TODO add GroupKeys type
 export default function Group<Datum, XScaleInput, YScaleInput>({
   horizontal,
   children,
+  ...rectProps
 }: GroupProps) {
   const {
     width,
@@ -41,7 +42,8 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
     [children],
   );
 
-  const withinGroupScale = useMemo(
+  //
+  const groupScale = useMemo(
     () =>
       scaleBand<string>({
         domain: [...dataKeys],
@@ -52,7 +54,7 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
   );
 
   // @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<Datum, XScaleInput, YScaleInput>) => {
     const nearestDatum = horizontal
@@ -66,16 +68,16 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
       : 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<Datum, XScaleInput, YScaleInput>({
       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<Datum, XScaleInput, YScaleInput>({
   }
 
   // @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 ? (
     <BarGroupHorizontal<unknown, string>
       data={combinedData}
@@ -143,12 +145,13 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
       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 <animated.g /> we might be able to make this animate on first render
           <VxGroup key={`bar-group-${barGroup.index}-${barGroup.y0}`} top={barGroup.y0}>
             <AnimatedBars
               bars={barGroup.bars}
@@ -157,6 +160,7 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
               width={bar => Math.abs(bar.width - scaledZeroPosition)}
               height={bar => bar.height}
               rx={2}
+              {...rectProps}
             />
           </VxGroup>
         ))
@@ -169,7 +173,7 @@ export default function Group<Datum, XScaleInput, YScaleInput>({
       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<Datum, XScaleInput, YScaleInput>({
               width={bar => bar.width}
               height={bar => Math.abs(scaledZeroPosition - bar.y)}
               rx={2}
+              {...rectProps}
             />
           </VxGroup>
         ))

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
@@ -16,14 +16,13 @@ const STACK_ACCESSOR = d => d.stack;
 export type GroupProps = {
   horizontal?: boolean;
   children: typeof BarSeries;
-};
+} & Omit<React.SVGProps<SVGRectElement>, 'x' | 'y' | 'width' | 'height' | 'ref'>;
 
 export default function Stack<Datum, XScaleInput, YScaleInput>({
   horizontal,
   children,
+  ...rectProps
 }: GroupProps) {
-  console.log('render Stack');
-
   const { xScale, yScale, colorScale, dataRegistry, registerData, unregisterData, height, margin } =
     (useContext(ChartContext) as ChartContextType<Datum, XScaleInput, YScaleInput>) || {};
 
@@ -36,6 +35,7 @@ export default function Stack<Datum, XScaleInput, YScaleInput>({
   // use a ref to the stacks for mouse movements
   const stacks = useRef<BarStackType<unknown, string>[] | null>(null);
 
+  // override the findNearestDatum logic
   const findNearestDatum = useCallback(
     (args: NearestDatumArgs<Datum, XScaleInput, YScaleInput>) => {
       if (!stacks.current) return;
@@ -107,6 +107,7 @@ export default function Stack<Datum, XScaleInput, YScaleInput>({
     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<Datum, XScaleInput, YScaleInput>({
     });
 
     registerData(dataToRegister);
+
+    // unregister data on unmount
     return () => unregisterData(Object.keys(dataToRegister));
   }, [
     horizontal,
@@ -166,7 +169,11 @@ export default function Stack<Datum, XScaleInput, YScaleInput>({
         // use this reference to find nearest mouse values
         stacks.current = barStacks;
         return barStacks.map((barStack, index) => (
-          <AnimatedBars key={`${index}-${barStack.bars.length}`} bars={barStack.bars} />
+          <AnimatedBars
+            key={`${index}-${barStack.bars.length}`}
+            bars={barStack.bars}
+            {...rectProps}
+          />
         ));
       }}
     </BarStackHorizontal>
@@ -186,7 +193,11 @@ export default function Stack<Datum, XScaleInput, YScaleInput>({
         // use this reference to find nearest mouse values
         stacks.current = barStacks;
         return barStacks.map((barStack, index) => (
-          <AnimatedBars key={`${index}-${barStack.bars.length}`} bars={barStack.bars} />
+          <AnimatedBars
+            key={`${index}-${barStack.bars.length}`}
+            bars={barStack.bars}
+            {...rectProps}
+          />
         ));
       }}
     </BarStack>

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
@@ -27,7 +27,7 @@ export default function withRegisteredData<
 ) {
   const WrappedSeriesComponent: FunctionComponent<BaseComponentProps> = 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 ? <BaseSeriesComponent {...props} /> : null;
+    return xScale && yScale && dataRegistry?.[dataKey]?.data === data ? (
+      <BaseSeriesComponent {...props} />
+    ) : null;
   };
 
   return WrappedSeriesComponent;