Add user props to top-level container or group

This moves the user props logic from the container component to the
top-level returned component, whether that is a container or a group
component. This allows the safe props to work in a standlone component,
or a component that is a child of another victory container or
VictoryChart component.

packages/victory-area/src/victory-area.js

@@ -108,13 +108,11 @@ class VictoryArea extends React.Component {
     }
 
     const children = this.renderContinuousData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-bar/src/victory-bar.js

@@ -116,13 +116,12 @@ class VictoryBar extends React.Component {
     }
 
     const children = this.renderData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-box-plot/src/victory-box-plot.js

@@ -309,13 +309,11 @@ class VictoryBoxPlot extends React.Component {
     }
 
     const children = this.renderBoxPlot(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-candlestick/src/victory-candlestick.js

@@ -296,13 +296,12 @@ class VictoryCandlestick extends React.Component {
     }
 
     const children = this.renderCandleData(props, this.shouldRenderDatum);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-core/src/victory-clip-container/victory-clip-container.js

@@ -2,6 +2,7 @@ import React from "react";
 import PropTypes from "prop-types";
 import * as CustomPropTypes from "../victory-util/prop-types";
 import * as Helpers from "../victory-util/helpers";
+import * as UserProps from "../victory-util/user-props";
 import { assign, defaults, isObject, uniqueId } from "lodash";
 import ClipPath from "../victory-primitives/clip-path";
 import Circle from "../victory-primitives/circle";
@@ -84,6 +85,7 @@ export default class VictoryClipContainer extends React.Component {
   }
 
   renderClippedGroup(props, clipId) {
+    const userProps = UserProps.getSafeUserProps(props);
     const {
       style,
       events,
@@ -106,7 +108,7 @@ export default class VictoryClipContainer extends React.Component {
     );
     return React.cloneElement(
       groupComponent,
-      { ...groupProps, "aria-label": props["aria-label"], tabIndex },
+      { ...groupProps, tabIndex, ...userProps },
       [clipComponent, ...React.Children.toArray(children)]
     );
   }

packages/victory-core/src/victory-util/user-props.js

@@ -1,3 +1,5 @@
+import * as React from "react";
+
 /*
   USER_PROPS_SAFELIST is to contain any string deemed safe for user props.
   The startsWidth array will contain the start of any accepted user-prop that 
@@ -79,3 +81,14 @@ export const getSafeUserProps = (props) => {
       })
   );
 };
+
+/**
+ * Wraps a component and adds safe user props
+ *
+ * @param {ReactNode} component: parent component
+ * @param {Object} props: props to be filtered
+ * @returns {ReactNode} modified component
+ */
+export const withSafeUserProps = (component, props) => {
+  return React.cloneElement(component, getSafeUserProps(props));
+};

packages/victory-errorbar/src/victory-errorbar.js

@@ -106,13 +106,12 @@ class VictoryErrorBar extends React.Component {
     }
 
     const children = this.renderData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-histogram/src/victory-histogram.js

@@ -122,14 +122,11 @@ export class VictoryHistogram extends React.Component {
 
     const children = this.renderData(props);
 
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(this.props)
-    );
-
-    return props.standalone
-      ? this.renderContainer(container, children)
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-line/src/victory-line.js

@@ -111,13 +111,12 @@ class VictoryLine extends React.Component {
     }
 
     const children = this.renderContinuousData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 export default addEvents(VictoryLine, options);

packages/victory-pie/src/victory-pie.js

@@ -256,13 +256,12 @@ class VictoryPie extends React.Component {
     }
 
     const children = this.renderData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-scatter/src/victory-scatter.js

@@ -99,13 +99,12 @@ class VictoryScatter extends React.Component {
     }
 
     const children = this.renderData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

packages/victory-voronoi/src/victory-voronoi.js

@@ -78,13 +78,12 @@ class VictoryVoronoi extends React.Component {
     }
 
     const children = this.renderData(props);
-    const container = React.cloneElement(
-      props.containerComponent,
-      UserProps.getSafeUserProps(props)
-    );
-    return props.standalone
-      ? this.renderContainer(container, children)
+
+    const component = props.standalone
+      ? this.renderContainer(props.containerComponent, children)
       : children;
+
+    return UserProps.withSafeUserProps(component, props);
   }
 }
 

test/jest/rendering-utils.js

@@ -1,6 +1,5 @@
 import { render } from "@testing-library/react";
-import * as React from "react";
 
 export const renderInSvg = (component) => {
-  return render(<svg>{component}</svg>);
+  return render(component, { wrapper: "svg" });
 };

test/jest/victory-area/victory-area.test.js

@@ -1,17 +1,43 @@
+import "@testing-library/jest-dom";
 import { fireEvent, render, screen } from "@testing-library/react";
-import { curveCatmullRom } from "victory-vendor/d3-shape";
 import { range } from "lodash";
 import React from "react";
 import { Area, VictoryArea } from "victory-area";
+import { VictoryChart } from "victory-chart";
+import { curveCatmullRom } from "victory-vendor/d3-shape";
 import { calculateD3Path } from "../../svg-test-helper";
 
 describe("components/victory-area", () => {
   describe("default component rendering", () => {
-    it("accepts user props", () => {
-      render(<VictoryArea data-testid="victory-area" aria-label="Chart" />);
+    it("attaches safe user props to the container component", () => {
+      render(
+        <VictoryArea
+          data-testid="victory-area"
+          aria-label="Chart"
+          unsafe-prop="test"
+        />
+      );
+
+      const container = screen.getByTestId("victory-area");
+      expect(screen.getByLabelText("Chart")).toBeDefined();
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.nodeName).toEqual("svg");
+    });
+
+    it("attaches safe user props to the group component if the component is rendered inside a VictoryChart", () => {
+      render(
+        <VictoryArea
+          data-testid="victory-area"
+          aria-label="Chart"
+          unsafe-prop="test"
+        />,
+        { wrapper: VictoryChart }
+      );
 
-      expect(screen.queryByTestId("victory-area")).toBeDefined();
+      const container = screen.getByTestId("victory-area");
       expect(screen.getByLabelText("Chart")).toBeDefined();
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.nodeName).toEqual("g");
     });
 
     it("renders an svg with the correct viewbox", () => {

test/jest/victory-bar/victory-bar.test.js

@@ -3,6 +3,7 @@
 import React from "react";
 import { render, fireEvent, screen } from "@testing-library/react";
 import { range } from "lodash";
+import { VictoryChart } from "victory-chart";
 import { VictoryBar, Bar } from "victory-bar";
 import { isBar, getBarHeight } from "../../svg-test-helper";
 import "@testing-library/jest-dom";
@@ -18,11 +19,26 @@ describe("components/victory-bar", () => {
         />
       );
 
-      expect(screen.getByTestId("victory-bar")).toBeDefined();
+      const container = screen.getByTestId("victory-bar");
       expect(screen.getByLabelText("Chart")).toBeDefined();
-      expect(screen.getByTestId("victory-bar")).not.toHaveAttribute(
-        "unsafe-prop"
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.tagName).toEqual("svg");
+    });
+
+    it("attaches safe user props to the group component if the component is rendered inside a VictoryChart", () => {
+      render(
+        <VictoryBar
+          data-testid="victory-bar"
+          aria-label="Chart"
+          unsafe-prop="test"
+        />,
+        { wrapper: VictoryChart }
       );
+
+      const container = screen.getByTestId("victory-bar");
+      expect(screen.getByLabelText("Chart")).toBeDefined();
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.tagName).toEqual("g");
     });
 
     it("renders an svg with the correct width and height", () => {

test/jest/victory-box-plot/victory-box-plot.test.js

@@ -1,8 +1,10 @@
 /*eslint-disable react/prop-types */
-import React from "react";
+import "@testing-library/jest-dom";
 import { render, screen } from "@testing-library/react";
+import React from "react";
 import { VictoryBoxPlot } from "victory-box-plot";
-import { LineSegment, Whisker, Border } from "victory-core";
+import { VictoryChart } from "victory-chart";
+import { Border, LineSegment, Whisker } from "victory-core";
 
 const TEST_GROUP_ID = "test-group-id";
 const dataset = [
@@ -26,12 +28,35 @@ const renderWithTestGroup = (data = dataset) => {
 
 describe("components/victory-box-plot", () => {
   describe("default component rendering", () => {
-    it("accepts user props", () => {
+    it("attaches safe user props to the container component", () => {
       render(
-        <VictoryBoxPlot data-testid="victory-boxplot" aria-label="Chart" />
+        <VictoryBoxPlot
+          data-testid="victory-boxplot"
+          aria-label="Chart"
+          unsafe-prop="test"
+        />
+      );
+
+      const container = screen.getByTestId("victory-boxplot");
+      expect(screen.getByLabelText("Chart")).toBeDefined();
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.nodeName).toEqual("svg");
+    });
+
+    it("attaches safe user props to the group component if the component is rendered inside a VictoryChart", () => {
+      render(
+        <VictoryBoxPlot
+          data-testid="victory-boxplot"
+          aria-label="Chart"
+          unsafe-prop="test"
+        />,
+        { wrapper: VictoryChart }
       );
 
+      const container = screen.getByTestId("victory-boxplot");
       expect(screen.getByLabelText("Chart")).toBeDefined();
+      expect(container).not.toHaveAttribute("unsafe-prop");
+      expect(container.nodeName).toEqual("g");
     });
 
     it("renders an svg with the correct width and height", () => {