diff --git a/src/cartesian/Brush.js b/src/cartesian/Brush.js
index d8828f3aec..e919b6557e 100644
--- a/src/cartesian/Brush.js
+++ b/src/cartesian/Brush.js
@@ -4,12 +4,11 @@
import React, { Component, PropTypes } from 'react';
import classNames from 'classnames';
import { scalePoint } from 'd3-scale';
-import pureRender from '../util/PureRender';
+import { shallowEqual } from '../util/PureRender';
import Layer from '../container/Layer';
import Text from '../component/Text';
import _ from 'lodash';
-@pureRender
class Brush extends Component {
static displayName = 'Brush';
@@ -31,7 +30,7 @@ class Brush extends Component {
tickFormatter: PropTypes.func,
affectedCharts: PropTypes.oneOf(['all', 'self', 'others']),
overlayChart: PropTypes.bool,
- onChange: PropTypes.func,
+ onChange: PropTypes.oneOfType([PropTypes.func, PropTypes.array]),
};
static defaultProps = {
@@ -69,6 +68,13 @@ class Brush extends Component {
componentWillReceiveProps(nextProps) {
const { data, width, x, travellerWidth, startIndex, endIndex } = this.props;
+ // when affectedCharts==='others', the props might not change for start/endIndex
+ // even though the underlying value does. Only update the state values
+ // if the props have changed from their prior values
+ if (startIndex !== nextProps.startIndex || endIndex !== nextProps.endIndex) {
+ this.setState({ startIndex, endIndex });
+ }
+
if (nextProps.data !== data) {
this.updateScale(nextProps);
this.setState({ startIndex, endIndex });
@@ -84,6 +90,14 @@ class Brush extends Component {
}
}
+ shouldComponentUpdate({ onChange, ...restNextProps }, nextState) {
+ // onChange could be an array which might be created new each time, so deep equal check it
+ const { onChange: onChangeOld, ...restOldProps } = this.props;
+
+ return !shallowEqual(onChange, onChangeOld) ||
+ !shallowEqual(restNextProps, restOldProps) || !shallowEqual(nextState, this.state);
+ }
+
componentWillUnmount() {
this.scale = null;
this.scaleValues = null;
@@ -124,6 +138,17 @@ class Brush extends Component {
};
}
+ /*
+ * Get the startIndex and endIndex from props, unless
+ * affectedCharts is others, in which case you need get it off of state
+ */
+ getCurrentIndex() {
+ if (this.props.affectedCharts === 'others') {
+ return { startIndex: this.state.startIndex, endIndex: this.state.endIndex };
+ }
+ return { startIndex: this.props.startIndex, endIndex: this.props.endIndex };
+ }
+
getTextOfTick(index) {
const { data, tickFormatter, dataKey } = this.props;
const text = (data[index] && dataKey) ? data[index][dataKey] : index;
@@ -196,15 +221,18 @@ class Brush extends Component {
endX: endX + delta,
});
+ const isIndexChange = !shallowEqual(newIndex, this.getCurrentIndex());
+
this.setState({
startX: startX + delta,
endX: endX + delta,
slideMoveStartX: e.pageX,
...newIndex,
}, () => {
- if (onChange) {
- onChange(newIndex);
- }
+ if (!isIndexChange) return;
+ if (_.isArray(onChange)) {
+ onChange.forEach((f) => f(newIndex));
+ } else { onChange(newIndex); }
});
}
@@ -234,14 +262,17 @@ class Brush extends Component {
params[movingTravellerId] = prevValue + delta;
const newIndex = this.getIndex(params);
+ const isIndexChange = !shallowEqual(newIndex, this.getCurrentIndex());
+
this.setState({
[movingTravellerId]: prevValue + delta,
brushMoveStartX: e.pageX,
...newIndex,
}, () => {
- if (onChange) {
- onChange(newIndex);
- }
+ if (!isIndexChange) return;
+ if (_.isArray(onChange)) {
+ onChange.forEach((f) => f(newIndex));
+ } else { onChange(newIndex); }
});
}
@@ -341,15 +372,10 @@ class Brush extends Component {
}
renderText() {
- const { y, height, travellerWidth, stroke, affectedCharts } = this.props;
- let { startIndex, endIndex } = this.props;
+ const { y, height, travellerWidth, stroke } = this.props;
const { startX, endX } = this.state;
- // if only other charts are affected, need to use our own state for the text
- // as we won't necessarily get new props from the chart
- if (affectedCharts === 'others') {
- ({ startIndex, endIndex } = this.state);
- }
+ const { startIndex, endIndex } = this.getCurrentIndex();
const offset = 5;
const style = {
diff --git a/src/chart/generateCategoricalChart.js b/src/chart/generateCategoricalChart.js
index a64de66dc3..30f2842818 100644
--- a/src/chart/generateCategoricalChart.js
+++ b/src/chart/generateCategoricalChart.js
@@ -861,8 +861,11 @@ const generateCategoricalChart = (ChartComponent, GraphicalChild) => {
overrideHeight = { height: offset.height };
}
+ const onChange = [this.handleBrushChangeForThis];
+ if (brushItem.props.onChange) onChange.push(brushItem.props.onChange);
+
return React.cloneElement(brushItem, {
- onChange: this.handleBrushChangeForThis,
+ onChange,
data,
x: offset.left,
y,
diff --git a/test/specs/chart/LineChartSpec.js b/test/specs/chart/LineChartSpec.js
index 49c9017805..3870af40b2 100644
--- a/test/specs/chart/LineChartSpec.js
+++ b/test/specs/chart/LineChartSpec.js
@@ -408,6 +408,32 @@ describe(' - affectedCharts prop', () => {
});
+ it('should call a supplied onChange function to Brush in addition to handleBrushChange', () => {
+
+ const onChangeSpy = sinon.spy();
+ const wrapper = mount(totalChart({ onChange: onChangeSpy }));
+
+ expect(onChangeSpy.callCount).to.equal(0);
+ // since props don't trickle down to Brush when affectedCharts="others"
+ // we can't cheat by using the Charts handleBrushChange method
+ // need to actually simulate mouse actions
+ const brush = wrapper.find(Brush).get(0);
+ brush.handleTravellerDown('startX', { pageX: brush.scale(0) });
+ brush.handleTravellerMove({ pageX: brush.scale(2) });
+ brush.handleUp();
+
+ expect(onChangeSpy.callCount).to.equal(1);
+ expect(onChangeSpy.getCall(0).args).to.eql([{ startIndex: 2, endIndex: 5 }]);
+
+ brush.handleTravellerDown('endX', { pageX: brush.scale(5) });
+ brush.handleTravellerMove({ pageX: brush.scale(4) });
+ brush.handleUp();
+
+ expect(onChangeSpy.callCount).to.equal(2);
+ expect(onChangeSpy.getCall(1).args).to.eql([{ startIndex: 2, endIndex: 4 }]);
+
+ });
+
});