-
Notifications
You must be signed in to change notification settings - Fork 70
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
Brush Interaction for XY-chart #117
Changes from 9 commits
97b6f2c
95108b4
5caf624
420090f
6e02d6f
3b597a7
d139626
5421253
375de16
061752c
c99ba69
34de8ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import { | |
isCrossHair, | ||
isReferenceLine, | ||
isSeries, | ||
isBrush, | ||
getChildWithName, | ||
numTicksForWidth, | ||
numTicksForHeight, | ||
|
@@ -91,7 +92,9 @@ class XYChart extends React.PureComponent { | |
this.handleClick = this.handleClick.bind(this); | ||
this.handleMouseLeave = this.handleMouseLeave.bind(this); | ||
this.handleMouseMove = this.handleMouseMove.bind(this); | ||
this.handleMouseDown = this.handleMouseDown.bind(this); | ||
this.handleContainerEvent = this.handleContainerEvent.bind(this); | ||
this.registerBrushStartEvent = this.registerBrushStartEvent.bind(this); | ||
} | ||
|
||
componentDidMount() { | ||
|
@@ -107,8 +110,11 @@ class XYChart extends React.PureComponent { | |
|
||
componentWillReceiveProps(nextProps) { | ||
let shouldComputeScales = false; | ||
// eslint-disable-next-line react/destructuring-assignment | ||
if (['width', 'height', 'children'].some(prop => this.props[prop] !== nextProps[prop])) { | ||
if ( | ||
['width', 'height', 'children'].some( | ||
prop => this.props[prop] !== nextProps[prop], // eslint-disable-line react/destructuring-assignment | ||
) | ||
) { | ||
shouldComputeScales = true; | ||
} | ||
if ( | ||
|
@@ -183,6 +189,16 @@ class XYChart extends React.PureComponent { | |
} | ||
} | ||
|
||
registerBrushStartEvent(event) { | ||
this.fireBrushStart = event; | ||
} | ||
|
||
handleMouseDown(event) { | ||
if (this.fireBrushStart) { | ||
this.fireBrushStart(event); | ||
} | ||
} | ||
|
||
handleMouseMove(args) { | ||
const { snapTooltipToDataX, snapTooltipToDataY, onMouseMove } = this.props; | ||
const isFocusEvent = args.event && args.event.type === 'focus'; | ||
|
@@ -259,6 +275,10 @@ class XYChart extends React.PureComponent { | |
const { numXTicks, numYTicks } = this.getNumTicks(innerWidth, innerHeight); | ||
const barWidth = xScale.barWidth || (xScale.bandwidth && xScale.bandwidth()) || 0; | ||
const CrossHairs = []; // ensure these are the top-most layer | ||
let hasBrush = false; | ||
let Brush; | ||
let xAxisOrientation; | ||
let yAxisOrientation; | ||
|
||
return ( | ||
innerWidth > 0 && | ||
|
@@ -283,6 +303,11 @@ class XYChart extends React.PureComponent { | |
const name = componentName(Child); | ||
if (isAxis(name)) { | ||
const styleKey = name[0].toLowerCase(); | ||
if (name === 'XAxis') { | ||
xAxisOrientation = Child.props.orientation; | ||
} else { | ||
yAxisOrientation = Child.props.orientation; | ||
} | ||
|
||
return React.cloneElement(Child, { | ||
innerHeight, | ||
|
@@ -322,6 +347,11 @@ class XYChart extends React.PureComponent { | |
return null; | ||
} else if (isReferenceLine(name)) { | ||
return React.cloneElement(Child, { xScale, yScale }); | ||
} else if (isBrush(name)) { | ||
hasBrush = true; | ||
Brush = Child; | ||
|
||
return null; | ||
} | ||
|
||
return Child; | ||
|
@@ -335,6 +365,7 @@ class XYChart extends React.PureComponent { | |
width={innerWidth} | ||
height={innerHeight} | ||
onClick={this.handleClick} | ||
onMouseDown={this.handleMouseDown} | ||
onMouseMove={this.handleMouseMove} | ||
onMouseLeave={this.handleMouseLeave} | ||
showVoronoi={showVoronoi} | ||
|
@@ -349,12 +380,26 @@ class XYChart extends React.PureComponent { | |
height={innerHeight} | ||
fill="transparent" | ||
fillOpacity={0} | ||
onMouseDown={this.handleMouseDown} | ||
onClick={this.handleContainerEvent} | ||
onMouseMove={this.handleContainerEvent} | ||
onMouseLeave={this.handleMouseLeave} | ||
/> | ||
)} | ||
|
||
{hasBrush && | ||
React.cloneElement(Brush, { | ||
xScale, | ||
yScale, | ||
innerHeight, | ||
innerWidth, | ||
margin, | ||
registerStartEvent: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add a comment about why this isn't needed for a series event trigger? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! In deed for area chart and bar chart, we do need to do like this. Will revise the code logic. |
||
eventTrigger === SERIES_TRIGGER ? null : this.registerBrushStartEvent, | ||
xAxisOrientation, | ||
yAxisOrientation, | ||
})} | ||
|
||
{tooltipData && | ||
CrossHairs.length > 0 && | ||
CrossHairs.map((CrossHair, i) => | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,3 +25,4 @@ export { PatternLines, PatternCircles, PatternWaves, PatternHexagons } from '@vx | |
export { withScreenSize, withParentSize, ParentSize } from '@vx/responsive'; | ||
export { default as withTheme } from './enhancer/withTheme'; | ||
export { chartTheme as theme } from '@data-ui/theme'; | ||
export { default as Brush } from './selection/Brush'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you think all of the brush components should go under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So here, I was thinking for a long term, will we keep the code here or we should move it to vx? The reason I put it in the utils is that I think now it is the temporary solution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you initially set
let Brush = null
and then just callBrush && React.cloneElement(Brush ...)
below to get rid of the extrahasBrush
var?