Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VictoryChart: adds an option for adding background for chart area #1558

Merged
merged 20 commits into from
May 11, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4a339c8
Added new Background primitive, set up methods in VictoryChart to ren…
maddles May 5, 2020
26554d9
Rendering background as first child in charts if background property …
maddles May 6, 2020
f41363a
Make sure background property remains on style prop for VictoryChart …
maddles May 6, 2020
6c6253a
Get the chart background rendering in correct range, fix bug with zer…
maddles May 6, 2020
5fdf2cf
Adds additional stories to show the background property on the style …
wsparsons May 6, 2020
1856706
React proptypes for Chart Background, Polar chart background
maddles May 6, 2020
0493017
Make sure getHeight and getRangeBounds return the right values in bac…
maddles May 6, 2020
e0e0f1f
Remove an unused import in Background, alphebetise props
maddles May 6, 2020
fc8cf42
Adds additional stories to show background style on circle and rect c…
wsparsons May 7, 2020
0142050
Removes uninvoked functions
wsparsons May 7, 2020
ce270ae
Adds background property to the style prop for VictoryChartProps inte…
wsparsons May 7, 2020
8ddb47c
Add separate props for VictoryChart background
maddles May 8, 2020
95cd939
Refactors backgroundProps to take into account the x and y coordinate…
wsparsons May 8, 2020
f0c280e
Adds a few more UI tests for chart background (stack, group, and hori…
maddles May 8, 2020
1e824ec
Undefined check for style in victory-chart
maddles May 8, 2020
bdd6128
Remove background styles from props passed to other children of chart…
maddles May 8, 2020
a28aa7d
Remove disused cx and cy props from Background, ensure that backgroun…
maddles May 8, 2020
226d9a2
Adds a few more demos to victory charts
wsparsons May 11, 2020
6e0f1b3
defaults object for background props in chart helper methods, export …
maddles May 11, 2020
ee88dbd
Remove disused import from chart js demo
maddles May 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions demo/js/components/victory-chart-demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,25 +180,40 @@ class App extends React.Component {
alignItems: "center",
justifyContent: "center"
};
const chartStyle = { parent: { border: "1px solid #ccc", margin: "2%", maxWidth: "40%" } };
const chartStyle = {
parent: { border: "1px solid #ccc", margin: "2%", maxWidth: "40%" }
};
const axisStyle = {
grid: { stroke: "grey", strokeWidth: 1 },
axis: { stroke: "transparent" },
ticks: { stroke: "transparent" },
tickLabels: { fill: "none" }
};

const bgStyle = {
background: { fill: "#e6e6ff" }
};

return (
<div className="demo">
<h1>VictoryChart</h1>
<div style={containerStyle}>
<VictoryChart style={chartStyle}>
<VictoryChart style={chartStyle} polar>
<VictoryScatter />
</VictoryChart>

<VictoryChart style={assign({}, chartStyle, bgStyle)}>
<VictoryScatter data={[{ x: -3, y: -3 }]} />
</VictoryChart>

<VictoryChart style={chartStyle} theme={dependentAxisTheme}>
<VictoryScatter />
</VictoryChart>

<VictoryChart style={chartStyle} theme={dependentAxisTheme}>
<VictoryScatter />
</VictoryChart>

<VictoryChart style={chartStyle} domainPadding={{ x: [0, 20] }}>
<VictoryAxis dependentAxis style={axisStyle} />
<VictoryAxis style={axisStyle} tickCount={6} />
Expand Down
9 changes: 7 additions & 2 deletions demo/ts/components/victory-chart-demo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,18 @@ class VictoryChartDemo extends React.Component<any, VictoryChartDemoState> {
alignItems: "center",
justifyContent: "center"
};
const chartStyle = { parent: { border: "1px solid #ccc", margin: "2%", maxWidth: "40%" } };
const axisStyle = {

const chartStyle: { [key: string]: React.CSSProperties } = {
parent: { border: "1px solid #ccc", margin: "2%", maxWidth: "40%" }
};

const axisStyle: { [key: string]: React.CSSProperties } = {
grid: { stroke: "grey", strokeWidth: 1 },
axis: { stroke: "transparent" },
ticks: { stroke: "transparent" },
tickLabels: { fill: "none" }
};

return (
<div className="demo">
<h1>VictoryChart</h1>
Expand Down
4 changes: 2 additions & 2 deletions demo/ts/components/victory-polar-axis-demo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { VictoryScatter } from "@packages/victory-scatter";
import { VictoryZoomContainer } from "@packages/victory-zoom-container";
import { VictoryVoronoiContainer } from "@packages/victory-voronoi-container";
import { random, range, keys } from "lodash";
import { VictoryTheme, VictoryLabel, VictoryStyleInterface } from "@packages/victory-core";
import { VictoryTheme, VictoryLabel } from "@packages/victory-core";

type multiAxisDataListType = {
strength?: number;
Expand Down Expand Up @@ -115,7 +115,7 @@ class App extends React.Component<any, VictoryPolarAxisState> {
justifyContent: "center"
};

const chartStyle: VictoryStyleInterface = {
const chartStyle: { [key: string]: React.CSSProperties } = {
parent: { border: "1px solid #ccc", margin: "2%", maxWidth: "40%" }
};

Expand Down
62 changes: 49 additions & 13 deletions packages/victory-chart/src/helper-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,37 @@ function getAxisProps(child, props, calculatedProps) {
};
}

function getBackgroundWithProps(props, calculatedProps) {
const backgroundElement = props.backgroundComponent;

const height = props.polar
? calculatedProps.range.y[1]
: calculatedProps.range.y[0] - calculatedProps.range.y[1];
const width = calculatedProps.range.x[1] - calculatedProps.range.x[0];

const xScale = props.horizontal
? calculatedProps.scale.y.range()[0]
: calculatedProps.scale.x.range()[0];
const yScale = props.horizontal
? calculatedProps.scale.x.range()[1]
: calculatedProps.scale.y.range()[1];

const xCoordinate = props.polar ? calculatedProps.origin.x : xScale;
const yCoordinate = props.polar ? calculatedProps.origin.y : yScale;

const backgroundProps = {
height,
polar: props.polar,
scale: calculatedProps.scale,
style: props.style.background,
x: xCoordinate,
y: yCoordinate,
width
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add scale to the list of backgroundProps.

};

return React.cloneElement(backgroundElement, backgroundProps);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use lodash defaults so that props set directly on the backgroundComponent are not nuked by props from the parent.

Like imagine the following scenario where a user wants the background to span the whole height of the svg but be confined to the chart width:

<VictoryChart
  height={300}
  width={300}
  style={{ background: { fill: "pink" } }}
  backgroundComponent={<Background y={0} height={300} />
 ...

The y value and height directly on Background should take prececence over what VictoryChart is trying to set.

I think the props should be defaults({}, backgroundElement.props, backgroundProps)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh great point

}

function getChildProps(child, props, calculatedProps) {
const axisChild = Axis.findAxisComponents([child]);
if (axisChild.length > 0) {
Expand All @@ -47,6 +78,7 @@ function getChildProps(child, props, calculatedProps) {

function getStyles(props) {
const styleProps = props.style && props.style.parent;

return {
parent: defaults({}, styleProps, {
height: "100%",
Expand Down Expand Up @@ -129,6 +161,7 @@ function getChildren(props, childComponents, calculatedProps) {
const { height, polar, theme, width } = props;
const { origin, horizontal } = calculatedProps;
const parentName = props.name || "chart";

return childComponents.map((child, index) => {
const role = child.type && child.type.role;
const style = Array.isArray(child.props.style)
Expand Down Expand Up @@ -158,21 +191,24 @@ function getChildren(props, childComponents, calculatedProps) {

const getChildComponents = (props, defaultAxes) => {
const childComponents = React.Children.toArray(props.children);
if (childComponents.length === 0) {
return [defaultAxes.independent, defaultAxes.dependent];
}
let newChildComponents = [...childComponents];

const axisComponents = {
dependent: Axis.getAxisComponentsWithParent(childComponents, "dependent"),
independent: Axis.getAxisComponentsWithParent(childComponents, "independent")
};
if (childComponents.length === 0) {
newChildComponents.push(defaultAxes.independent, defaultAxes.dependent);
} else {
const axisComponents = {
dependent: Axis.getAxisComponentsWithParent(childComponents, "dependent"),
independent: Axis.getAxisComponentsWithParent(childComponents, "independent")
};

if (axisComponents.dependent.length === 0 && axisComponents.independent.length === 0) {
return props.prependDefaultAxes
? [defaultAxes.independent, defaultAxes.dependent].concat(childComponents)
: childComponents.concat([defaultAxes.independent, defaultAxes.dependent]);
if (axisComponents.dependent.length === 0 && axisComponents.independent.length === 0) {
newChildComponents = props.prependDefaultAxes
? [defaultAxes.independent, defaultAxes.dependent].concat(newChildComponents)
: newChildComponents.concat([defaultAxes.independent, defaultAxes.dependent]);
}
}
return childComponents;

return newChildComponents;
};

const getDomain = (props, axis, childComponents) => {
Expand Down Expand Up @@ -252,4 +288,4 @@ const createStringMap = (props, childComponents) => {
return { x, y };
};

export { getChildren, getCalculatedProps, getChildComponents };
export { getBackgroundWithProps, getChildren, getCalculatedProps, getChildComponents };
15 changes: 8 additions & 7 deletions packages/victory-chart/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,32 @@
import * as React from "react";
import {
CategoryPropType,
EventPropTypeInterface,
DomainPropType,
EventPropTypeInterface,
StringOrNumberOrCallback,
VictoryCommonProps,
VictoryStyleInterface
VictoryStyleInterface,
VictoryStyleObject
} from "victory-core";

export type AxesType = {
independent?: React.ReactElement;
dependent?: React.ReactElement;
independent?: React.ReactElement;
};

export interface VictoryChartProps extends VictoryCommonProps {
defaultAxes?: AxesType;
defaultPolarAxes?: AxesType;
categories?: CategoryPropType;
children?: React.ReactNode | React.ReactNode[];
defaultAxes?: AxesType;
defaultPolarAxes?: AxesType;
domain?: DomainPropType;
endAngle?: number;
events?: EventPropTypeInterface<string, string[] | number[] | string | number>[];
eventKey?: StringOrNumberOrCallback;
events?: EventPropTypeInterface<string, string[] | number[] | string | number>[];
innerRadius?: number;
prependDefaultAxes?: boolean;
startAngle?: number;
style?: Pick<VictoryStyleInterface, "parent">;
style?: Pick<VictoryStyleInterface, "parent"> & { background?: VictoryStyleObject };
}

export class VictoryChart extends React.Component<VictoryChartProps, any> {}
22 changes: 20 additions & 2 deletions packages/victory-chart/src/victory-chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { defaults, assign, isEmpty } from "lodash";
import PropTypes from "prop-types";
import React from "react";
import {
Background,
Helpers,
VictoryContainer,
VictoryTheme,
Expand All @@ -12,7 +13,12 @@ import {
import { VictorySharedEvents } from "victory-shared-events";
import { VictoryAxis } from "victory-axis";
import { VictoryPolarAxis } from "victory-polar-axis";
import { getChildComponents, getCalculatedProps, getChildren } from "./helper-methods";
import {
getBackgroundWithProps,
getChildComponents,
getCalculatedProps,
getChildren
} from "./helper-methods";
import isEqual from "react-fast-compare";

const fallbackProps = {
Expand All @@ -26,6 +32,7 @@ export default class VictoryChart extends React.Component {

static propTypes = {
...CommonProps.baseProps,
backgroundComponent: PropTypes.element,
children: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.node), PropTypes.node]),
defaultAxes: PropTypes.shape({
independent: PropTypes.element,
Expand All @@ -42,6 +49,7 @@ export default class VictoryChart extends React.Component {
};

static defaultProps = {
backgroundComponent: <Background />,
containerComponent: <VictoryContainer />,
defaultAxes: {
independent: <VictoryAxis />,
Expand Down Expand Up @@ -84,10 +92,19 @@ export default class VictoryChart extends React.Component {
getNewChildren(props, childComponents, calculatedProps) {
const children = getChildren(props, childComponents, calculatedProps);
const getAnimationProps = Wrapper.getAnimationProps.bind(this);
return children.map((child, index) => {

const newChildren = children.map((child, index) => {
const childProps = assign({ animate: getAnimationProps(props, child, index) }, child.props);
return React.cloneElement(child, childProps);
});

if (props.style && props.style.background) {
const backgroundComponent = getBackgroundWithProps(props, calculatedProps);

newChildren.unshift(backgroundComponent);
}

return newChildren;
}

renderContainer(containerComponent, props) {
Expand Down Expand Up @@ -134,6 +151,7 @@ export default class VictoryChart extends React.Component {
? this.renderContainer(containerComponent, containerProps)
: groupComponent;
const events = Wrapper.getAllEvents(props);

if (!isEmpty(events)) {
return (
<VictorySharedEvents
Expand Down
11 changes: 11 additions & 0 deletions packages/victory-core/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,17 @@ export class VictoryPortal extends React.Component<VictoryPortalProps, any> {}

// #region Victory Primitives

export interface BackgroundProps extends VictoryCommonPrimitiveProps {
circleComponent?: React.ReactElement;
height?: number;
rectComponent?: React.ReactElement;
width?: number;
x?: number;
y?: number;
}

export class Background extends React.Component<BackgroundProps> {}

export interface VictoryPointProps extends VictoryCommonPrimitiveProps {
datum?: any;
getPath?: Function;
Expand Down
1 change: 1 addition & 0 deletions packages/victory-core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export { default as VictoryTheme } from "./victory-theme/victory-theme";
export { default as VictoryPortal } from "./victory-portal/victory-portal";
export { default as Portal } from "./victory-portal/portal";
export { default as Arc } from "./victory-primitives/arc";
export { default as Background } from "./victory-primitives/background";
Copy link
Contributor

@boygirl boygirl May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main victory package should also reexport the new Background primitive

export { default as Border, default as Box } from "./victory-primitives/border";
export { default as ClipPath } from "./victory-primitives/clip-path";
export { default as LineSegment } from "./victory-primitives/line-segment";
Expand Down
47 changes: 47 additions & 0 deletions packages/victory-core/src/victory-primitives/background.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from "react";
import PropTypes from "prop-types";
import CommonProps from "../victory-util/common-props";
import Rect from "./rect";
import Circle from "./circle";

const Background = (props) => {
return props.polar
? React.cloneElement(props.circleComponent, {
...props.events,
style: props.style,
role: props.role,
shapeRendering: props.shapeRendering,
cx: props.x,
cy: props.y,
r: props.height
})
: React.cloneElement(props.rectComponent, {
...props.events,
style: props.style,
role: props.role,
shapeRendering: props.shapeRendering,
x: props.x,
y: props.y,
width: props.width,
height: props.height
});
};

Background.propTypes = {
...CommonProps.primitiveProps,
circleComponent: PropTypes.element,
height: PropTypes.number,
rectComponent: PropTypes.element,
width: PropTypes.number,
x: PropTypes.number,
wsparsons marked this conversation as resolved.
Show resolved Hide resolved
y: PropTypes.number
};

Background.defaultProps = {
circleComponent: <Circle />,
rectComponent: <Rect />,
role: "presentation",
shapeRendering: "auto"
};

export default Background;
Loading