Merge pull request 'BarGraph -- update to work with a ChartContainer' (#118) from loewy/bar-graph-update-for-chart-container into master

Reviewed-on: railbird/railbird-mobile#118
Reviewed-by: Kat Huang <kkathuang@gmail.com>

src/component/charts/bar-graph/bar-graph.tsx

@@ -1,137 +1,45 @@
-import * as scale from "d3-scale";
 import React from "react";
-import { View } from "react-native";
+import { BarChart } from "react-native-svg-charts";
-import { BarChart, XAxis, YAxis } from "react-native-svg-charts";
 
-import { GraphData, YAxisProps } from "../graph-types";
+import { CommonProps, withChartType } from "../graph-types";
-import { useGraphData } from "../use-graph-data";
 
-import ChartLabel from "../chart-label/chart-label";
 import { graphStyles } from "../chart-styles";
-import ChartView from "../chart-view";
 import { CustomBars } from "../custom-bars";
 import { CustomGrid } from "../custom-grid";
 
-interface Props {
-	data: GraphData;
-	height?: number;
-	spacingInner?: number;
-	spacingOuter?: number;
-	contentInset?: { top: number; bottom: number };
-	min?: number;
-	numberOfTicks?: number;
-	barColors?: Array<string>;
-	useCommonScale?: boolean;
-	yAxisProps?: YAxisProps;
-	testID?: string;
-}
-
 // TODO: #43 #31 separate PR will update useGraphData to take into account useCommonScale
 // eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
-export const BarGraph: React.FC<Props> = ({
+const BarGraphBase: React.FC<CommonProps> = ({ ...chartComponentProps }) => {
-	data,
-	useCommonScale = false,
-	testID,
-	...props
-}) => {
-	if (!data) {
-		return null;
-	} // TODO:#38
 	const {
-		xValues,
 		yData,
-		yAxisRightLabels,
+		xValues,
-		yAxisLeftLabels,
+		min,
-		defaultProps: {
+		contentInset,
-			height,
+		numberOfTicks,
 		spacingInner,
 		spacingOuter,
-			contentInset,
-			min,
-			numberOfTicks,
 		barColors,
-			yAxisProps: {
+	} = chartComponentProps;
-				maxLeftYAxisValue,
-				maxRightYAxisValue,
-				formatRightYAxisLabel,
-				formatLeftYAxisLabel,
-			},
-		},
-		// Proper error/loading handling from useQueryHandler can work with this rule #38
-		// eslint-disable-next-line react-hooks/rules-of-hooks
-	} = useGraphData(data, { includeColors: false, ...props });
-
-	// TODO: Will come from BE & destructured / color assigned in useGraphData
-	const yLabels = [
-		{ displayName: "Shots Taken", axis: "LEFT" as "LEFT", color: barColors[0] },
-		{
-			displayName: "Make Percentage",
-			axis: "RIGHT" as "RIGHT",
-			color: barColors[1],
-		},
-	];
-	const title = "Shots Taken / Make Percentage by Cut Angle";
 
 	return (
-		<ChartView>
+		<>
-			<ChartLabel title={title} yLabels={yLabels} />
-			<View
-				style={[graphStyles.rowContainer, { height: height }]}
-				testID={`bar-graph-${testID}`}
-			>
-				<YAxis
-					data={yAxisLeftLabels.values}
-					contentInset={contentInset}
-					svg={graphStyles.yAxisFontStyle}
-					style={graphStyles.yAxisLeftPadding}
-					min={min}
-					max={maxLeftYAxisValue}
-					numberOfTicks={numberOfTicks}
-					formatLabel={formatLeftYAxisLabel}
-				/>
-
-				<View style={graphStyles.flex}>
 			<BarChart
 				style={graphStyles.flex}
 				data={yData}
 				gridMin={min}
-						numberOfTicks={numberOfTicks} // rethink numberOfTicks, it should be determined automatically if we do our y axis scaling properly
+				numberOfTicks={numberOfTicks} // TODO: #31 rethink numberOfTicks, it should be determined automatically if we do our y axis scaling properly
-						yAccessor={({ item }) =>
+				yAccessor={({ item }) => (item as unknown as { value: number }).value}
-							(item as unknown as { value: number }).value
-						}
 				contentInset={contentInset}
 				spacingInner={spacingInner}
 				spacingOuter={spacingOuter}
 			>
 				<CustomGrid />
-						<CustomBars
+				<CustomBars barData={yData} xValues={xValues} barColors={barColors} />
-							barData={yData}
-							xValues={xValues}
-							barColors={barColors}
-						/>
 			</BarChart>
-					<XAxis
+		</>
-						data={xValues.map((_, index) => index)}
-						formatLabel={(_, index) => xValues[index]}
-						style={graphStyles.xAxisMarginTop}
-						svg={graphStyles.xAxisFontStyle}
-						scale={scale.scaleBand}
-					/>
-				</View>
-
-				<YAxis
-					data={yAxisRightLabels?.values ?? yAxisLeftLabels.values}
-					contentInset={contentInset}
-					svg={graphStyles.yAxisFontStyle}
-					style={graphStyles.yAxisRightPadding}
-					min={min}
-					max={maxRightYAxisValue}
-					numberOfTicks={numberOfTicks}
-					formatLabel={formatRightYAxisLabel}
-				/>
-			</View>
-		</ChartView>
 	);
 };
 
+const BarGraph = withChartType(BarGraphBase, "bar");
+
 export default BarGraph;

src/component/charts/container/chart-container.tsx

@@ -1,3 +1,4 @@
+import * as scale from "d3-scale";
 import React from "react";
 import { View } from "react-native";
 import { XAxis, YAxis } from "react-native-svg-charts";
@@ -29,6 +30,9 @@ const ChartContainer: React.FC<ChartContainerProps> = ({
 			min,
 			numberOfTicks,
 			lineStrokeWidth,
+			spacingInner,
+			spacingOuter,
+			barColors,
 			yAxisProps: {
 				maxLeftYAxisValue,
 				maxRightYAxisValue,
@@ -38,9 +42,12 @@ const ChartContainer: React.FC<ChartContainerProps> = ({
 		},
 		// Proper error/loading handling from useQueryHandler can work with this rule #38
 		// eslint-disable-next-line react-hooks/rules-of-hooks
-	} = useGraphData(data, props);
+	} = useGraphData(data, {
+		includeColors: ChartComponent.chartType === "bar" ? false : true,
+		...props,
+	});
 
-	// TODO: This could be done by the hook since it already handles spreading props.
+	// TODO: #31 This could be done by the hook since it already handles spreading props.
 	// Depending on if we want a context provider for Charts, that could be another way to handle it
 	const chartComponentProps = {
 		yData,
@@ -49,6 +56,9 @@ const ChartContainer: React.FC<ChartContainerProps> = ({
 		min,
 		contentInset,
 		numberOfTicks,
+		spacingInner,
+		spacingOuter,
+		barColors,
 	};
 	return (
 		<ChartView>
@@ -69,13 +79,17 @@ const ChartContainer: React.FC<ChartContainerProps> = ({
 				<View style={graphStyles.flex}>
 					<ChartComponent {...chartComponentProps} />
 					<XAxis
-						data={xValues.map((_, index: number) => index)} // TODO: update when useGraphHook returns explicit display values
+						data={xValues.map((_, index: number) => index)} // TODO: #31 update when useGraphHook returns explicit display values
 						formatLabel={(_, index) => xValues[index]}
 						style={[graphStyles.xAxisMarginTop]}
 						svg={graphStyles.xAxisFontStyle}
-						numberOfTicks={numberOfTicks} // need a dynamic way of setting number of ticks
+						{...(ChartComponent.chartType === "bar" && {
-						contentInset={graphStyles.horizontalInset}
+							scale: scale.scaleBand,
-						// scale={scale.scaleBand} // Need to know the type of ChartComponent
+						})}
+						{...(ChartComponent.chartType === "line" && {
+							numberOfTicks: numberOfTicks,
+							contentInset: graphStyles.horizontalInset,
+						})}
 					/>
 				</View>
 				<YAxis

src/component/charts/graph-types.ts

@@ -77,6 +77,11 @@ export interface CommonProps {
 	yData?: { data: { value: number }[]; svg: { fill: string } }[];
 	xValues?: XValue[];
 }
+
+type ChartComponentWithStatic = React.ComponentType<CommonProps> & {
+	chartType?: string;
+};
+
 /**
  * Common properties for graph render components.
  *
@@ -84,6 +89,22 @@ export interface CommonProps {
  * @property {React.ComponentType<CommonProps>} ChartComponent - The graph component to render
  */
 export interface ChartContainerProps extends CommonProps {
-	ChartComponent: React.ComponentType<CommonProps>;
+	ChartComponent: ChartComponentWithStatic;
 	data: GraphData;
 }
+
+/**
+ * Enhances a component with a `chartType` property.
+ *
+ * @param Component - The component to enhance.
+ * @param chartType - The type of chart.
+ * @returns The enhanced component.
+ */
+export function withChartType<T extends React.ComponentType<any>>(
+	Component: T,
+	chartType: string,
+): T {
+	(Component as React.ComponentType<any> & { chartType?: string }).chartType =
+		chartType;
+	return Component;
+}

src/component/charts/line-graph/line-graph.tsx

@@ -3,11 +3,11 @@ import React from "react";
 import { LineChart } from "react-native-svg-charts";
 import { graphStyles } from "../chart-styles";
 import { CustomGrid } from "../custom-grid";
-import { CommonProps } from "../graph-types";
+import { CommonProps, withChartType } from "../graph-types";
 
 // TODO: #43 #31 separate PR will update useGraphData to take into account useCommonScale
 // eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
-const LineGraph: React.FC<CommonProps> = ({ ...chartComponentProps }) => {
+const LineGraphBase: React.FC<CommonProps> = ({ ...chartComponentProps }) => {
 	const { yData, xValues, lineStrokeWidth, min, contentInset, numberOfTicks } =
 		chartComponentProps;
 	return (
@@ -29,4 +29,6 @@ const LineGraph: React.FC<CommonProps> = ({ ...chartComponentProps }) => {
 	);
 };
 
+const LineGraph = withChartType(LineGraphBase, "line");
+
 export default LineGraph;

test/component/bar-graph.test.tsx

@@ -1,20 +0,0 @@
-import { render } from "@testing-library/react-native";
-import React from "react";
-import BarGraph from "../../src/component/charts/bar-graph/bar-graph";
-import { graph_data_two_measures } from "../mock/charts/mock-data";
-
-describe("BarGraph Component Tests", () => {
-	it("renders correctly with data", () => {
-		const { getByTestId } = render(
-			<BarGraph data={graph_data_two_measures} testID="1" />,
-		);
-		expect(getByTestId(`bar-graph-1`)).toBeTruthy();
-	});
-
-	it("does not render without data", () => {
-		// Have to ts-ignore to test null data conditions
-		// @ts-ignore
-		const { queryByTestId } = render(<BarGraph testID="2" />);
-		expect(queryByTestId(`bar-graph-2`)).toBeNull();
-	});
-});

test/component/chart-container.test.tsx

@@ -1,8 +1,12 @@
 import { render } from "@testing-library/react-native";
 import React from "react";
+import BarGraph from "../../src/component/charts/bar-graph/bar-graph";
 import ChartContainer from "../../src/component/charts/container/chart-container";
 import LineGraph from "../../src/component/charts/line-graph/line-graph";
-import { line_chart_two_y_data } from "../mock/charts/mock-data";
+import {
+	graph_data_two_measures,
+	line_chart_two_y_data,
+} from "../mock/charts/mock-data";
 describe("ChartContainer Component Tests", () => {
 	it("renders correctly with data -- line graph", () => {
 		const { getByTestId } = render(
@@ -14,6 +18,16 @@ describe("ChartContainer Component Tests", () => {
 		);
 		expect(getByTestId(`chart-container-1`)).toBeTruthy();
 	});
+	it("renders correctly with data -- bar graph", () => {
+		const { getByTestId } = render(
+			<ChartContainer
+				data={graph_data_two_measures}
+				testID="1"
+				ChartComponent={BarGraph}
+			/>,
+		);
+		expect(getByTestId(`chart-container-1`)).toBeTruthy();
+	});
 
 	it("does not render without data props", () => {
 		// Have to ts-ignore to test null data conditions