Make sure D3Base always updates when width changes (https://github.com/woocommerce/woocommerce-admin/pull/961)

* Make sure D3Base always updates when props change

* Fix JS error when resizing the window

* d3Chart cleanup

* Update tests

* Update d3Base also when drawChart prop changes

* Save height and width without margins in params

* Remove resize listeners inside d3Base

* Remove unused param
This commit is contained in:
Albert Juhé Lluveras 2018-11-29 19:01:00 -06:00 committed by GitHub
parent 109592ebc9
commit 9b0c767749
3 changed files with 45 additions and 109 deletions

View File

@ -6,7 +6,7 @@
import classNames from 'classnames';
import PropTypes from 'prop-types';
import { Component, createRef } from '@wordpress/element';
import { isEmpty, isEqual } from 'lodash';
import { isEqual } from 'lodash';
import { select as d3Select } from 'd3-selection';
/**
@ -26,71 +26,32 @@ import './style.scss';
* phase' (i.e. in 'componentDidMount' and 'componentDidUpdate' methods).
*/
export default class D3Base extends Component {
static propTypes = {
className: PropTypes.string,
data: PropTypes.any, // required to detect changes in data
drawChart: PropTypes.func.isRequired,
getParams: PropTypes.func.isRequired,
type: PropTypes.string,
};
constructor( props ) {
super( props );
state = {
data: null,
params: null,
drawChart: null,
getParams: null,
type: null,
};
chartRef = createRef();
static getDerivedStateFromProps( nextProps, prevState ) {
let state = {};
if ( ! isEqual( nextProps.data, prevState.data ) ) {
state = { ...state, data: nextProps.data };
}
if ( ! isEqual( nextProps.drawChart, prevState.drawChart ) ) {
state = { ...state, drawChart: nextProps.drawChart };
}
if ( ! isEqual( nextProps.getParams, prevState.getParams ) ) {
state = { ...state, getParams: nextProps.getParams };
}
if ( nextProps.type !== prevState.type ) {
state = { ...state, type: nextProps.type };
}
if ( ! isEmpty( state ) ) {
return { ...state, params: null };
}
return null;
this.chartRef = createRef();
}
componentDidMount() {
window.addEventListener( 'resize', this.updateParams );
this.drawChart();
this.drawUpdatedChart();
}
shouldComponentUpdate( nextProps, nextState ) {
shouldComponentUpdate( nextProps ) {
return (
( nextState.params !== null && ! isEqual( this.state.params, nextState.params ) ) ||
! isEqual( this.state.data, nextState.data ) ||
this.state.type !== nextState.type
this.props.className !== nextProps.className ||
! isEqual( this.props.data, nextProps.data ) ||
this.props.drawChart !== nextProps.drawChart ||
this.props.height !== nextProps.height ||
this.props.type !== nextProps.type ||
this.props.width !== nextProps.width
);
}
componentDidUpdate() {
this.drawChart();
this.drawUpdatedChart();
}
componentWillUnmount() {
window.removeEventListener( 'resize', this.updateParams );
this.deleteChart();
}
@ -103,19 +64,14 @@ export default class D3Base extends Component {
/**
* Renders the chart, or triggers a rendering by updating the list of params.
*/
drawChart() {
if ( ! this.state.params ) {
this.updateParams();
return;
}
drawUpdatedChart() {
const { drawChart } = this.props;
const svg = this.getContainer();
this.props.drawChart( svg, this.state.params );
drawChart( svg );
}
getContainer() {
const { className } = this.props;
const { width, height } = this.state.params;
const { className, height, width } = this.props;
this.deleteChart();
@ -133,14 +89,16 @@ export default class D3Base extends Component {
return svg.append( 'g' );
}
updateParams = () => {
const params = this.state.getParams( this.chartRef.current );
this.setState( { params } );
};
render() {
return (
<div className={ classNames( 'd3-base', this.props.className ) } ref={ this.chartRef } />
);
}
}
D3Base.propTypes = {
className: PropTypes.string,
data: PropTypes.any, // required to detect changes in data
drawChart: PropTypes.func.isRequired,
type: PropTypes.string,
};

View File

@ -15,13 +15,12 @@ describe( 'D3base', () => {
const shallowWithoutLifecycle = arg => shallow( arg, { disableLifecycleMethods: true } );
test( 'should have d3Base class', () => {
const base = shallowWithoutLifecycle( <D3Base drawChart={ noop } getParams={ noop } /> );
const base = shallowWithoutLifecycle( <D3Base drawChart={ noop } /> );
expect( base.find( '.d3-base' ) ).toHaveLength( 1 );
} );
test( 'should render an svg', () => {
const getParams = () => ( { width: 100, height: 100 } );
const base = mount( <D3Base drawChart={ noop } getParams={ getParams } /> );
const base = mount( <D3Base drawChart={ noop } height="100" width="100" /> );
expect( base.render().find( 'svg' ) ).toHaveLength( 1 );
} );
@ -29,22 +28,7 @@ describe( 'D3base', () => {
const drawChart = svg => {
return svg.append( 'circle' );
};
const getParams = () => ( {
width: 100,
height: 100,
} );
const base = mount( <D3Base drawChart={ drawChart } getParams={ getParams } /> );
expect( base.render().find( 'circle' ) ).toHaveLength( 1 );
} );
test( 'should pass a property of getParams output to drawChart function', () => {
const getParams = () => ( {
tagName: 'circle',
} );
const drawChart = ( svg, params ) => {
return svg.append( params.tagName );
};
const base = mount( <D3Base drawChart={ drawChart } getParams={ getParams } /> );
const base = mount( <D3Base drawChart={ drawChart } height="100" width="100" /> );
expect( base.render().find( 'circle' ) ).toHaveLength( 1 );
} );
} );

View File

@ -47,17 +47,13 @@ class D3Chart extends Component {
this.state = {
allData: this.getAllData( props ),
type: props.type,
width: props.width,
};
this.tooltipRef = createRef();
}
componentDidUpdate( prevProps, prevState ) {
const { type, width } = this.props;
const { type } = this.props;
/* eslint-disable react/no-did-update-set-state */
if ( width !== prevProps.width ) {
this.setState( { width } );
}
const nextAllData = this.getAllData( this.props );
if ( ! isEqual( [ ...nextAllData ].sort(), [ ...prevState.allData ].sort() ) ) {
this.setState( { allData: nextAllData } );
@ -74,27 +70,27 @@ class D3Chart extends Component {
return [ ...props.data, ...orderedKeys ];
}
drawChart( node, params ) {
drawChart( node ) {
const { data, margin, type } = this.props;
const params = this.getParams();
const adjParams = Object.assign( {}, params, {
height: params.adjHeight,
width: params.adjWidth,
tooltip: d3Select( this.tooltipRef.current ),
valueType: params.valueType,
} );
const g = node
.attr( 'id', 'chart' )
.append( 'g' )
.attr( 'transform', `translate(${ margin.left },${ margin.top })` );
const adjParams = Object.assign( {}, params, {
height: params.height - margin.top - margin.bottom,
width: params.width - margin.left - margin.right,
tooltip: d3Select( this.tooltipRef.current ),
valueType: params.valueType,
} );
drawAxis( g, adjParams );
type === 'line' && drawLines( g, data, adjParams );
type === 'bar' && drawBars( g, data, adjParams );
return node;
}
getParams( node ) {
getParams() {
const {
colorScheme,
data,
@ -109,16 +105,14 @@ class D3Chart extends Component {
tooltipValueFormat,
tooltipTitle,
type,
width,
xFormat,
x2Format,
yFormat,
valueType,
} = this.props;
const { width } = this.state;
const calculatedWidth = width && width > 0 ? width : node.offsetWidth;
const calculatedHeight = height || node.offsetHeight;
const adjHeight = calculatedHeight - margin.top - margin.bottom;
const adjWidth = calculatedWidth - margin.left - margin.right;
const adjHeight = height - margin.top - margin.bottom;
const adjWidth = width - margin.left - margin.right;
const uniqueKeys = getUniqueKeys( data );
const newOrderedKeys = orderedKeys || getOrderedKeys( data, uniqueKeys );
const lineData = getLineData( data, newOrderedKeys );
@ -130,9 +124,10 @@ class D3Chart extends Component {
const xScale = getXScale( uniqueDates, adjWidth );
const xTicks = getXTicks( uniqueDates, adjWidth, mode, interval );
return {
adjHeight,
adjWidth,
colorScheme,
dateSpaces: getDateSpaces( data, uniqueDates, adjWidth, xLineScale ),
height: calculatedHeight,
line: getLine( xLineScale, yScale ),
lineData,
margin,
@ -146,7 +141,6 @@ class D3Chart extends Component {
type,
uniqueDates,
uniqueKeys,
width: calculatedWidth,
xFormat: getFormatter( xFormat, d3TimeFormat ),
x2Format: getFormatter( x2Format, d3TimeFormat ),
xGroupScale: getXGroupScale( orderedKeys, xScale ),
@ -174,9 +168,9 @@ class D3Chart extends Component {
className={ classNames( this.props.className ) }
data={ this.state.allData }
drawChart={ this.drawChart }
getParams={ this.getParams }
height={ this.props.height }
type={ this.state.type }
width={ this.state.width }
width={ this.props.width }
/>
<div className="d3-chart__tooltip" ref={ this.tooltipRef } />
</div>