Skip to content

Commit

Permalink
fix(brushing): enable mouseup event outside chart element (#120)
Browse files Browse the repository at this point in the history
This PR add the mouseup listener on the window element at the mousedown event for bushing and remove it after the brush is end. The PR also disabled the tooltip when brushing.

fix #119
  • Loading branch information
markov00 authored Mar 26, 2019
1 parent cc0d9dd commit 77d62f6
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 23 deletions.
4 changes: 3 additions & 1 deletion src/components/react_canvas/reactive_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ class Chart extends React.Component<ReactiveChartProps, ReactiveChartState> {
return <Rect x={x} y={y} width={width} height={height} fill="gray" opacity={0.6} />;
}
onStartBrusing = (event: { evt: MouseEvent }) => {
window.addEventListener('mouseup', this.onEndBrushing);
this.props.chartStore!.onBrushStart();
const { brushExtent } = this.props.chartStore!;
const point = getPoint(event.evt, brushExtent);
this.setState(() => ({
Expand All @@ -202,6 +204,7 @@ class Chart extends React.Component<ReactiveChartProps, ReactiveChartState> {
}));
}
onEndBrushing = () => {
window.removeEventListener('mouseup', this.onEndBrushing);
const { brushStart, brushEnd } = this.state;
this.props.chartStore!.onBrushEnd(brushStart, brushEnd);
this.setState(() => ({
Expand Down Expand Up @@ -254,7 +257,6 @@ class Chart extends React.Component<ReactiveChartProps, ReactiveChartState> {
if (isBrushEnabled) {
brushProps = {
onMouseDown: this.onStartBrusing,
onMouseUp: this.onEndBrushing,
onMouseMove: this.onBrushing,
};
}
Expand Down
52 changes: 44 additions & 8 deletions src/state/chart_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { DataSeriesColorsValues } from '../lib/series/series';
import { AxisSpec, BarSeriesSpec, Position } from '../lib/series/specs';
import { getAxisId, getGroupId, getSpecId } from '../lib/utils/ids';
import { TooltipType, TooltipValue } from '../lib/utils/interactions';
import { ScaleBand } from '../lib/utils/scales/scale_band';
import { ScaleContinuous } from '../lib/utils/scales/scale_continuous';
import { ScaleType } from '../lib/utils/scales/scales';
import { ChartStore } from './chart_state';

Expand Down Expand Up @@ -345,11 +347,14 @@ describe('Chart Store', () => {
store.chartDimensions.left = 10;

store.onBrushEndListener = undefined;
store.onBrushStart();
expect(store.isBrushing.get()).toBe(false);
store.onBrushEnd(start, end1);
expect(brushEndListener).not.toBeCalled();

store.setOnBrushEndListener(brushEndListener);

store.onBrushStart();
expect(store.isBrushing.get()).toBe(true);
store.onBrushEnd(start, start);
expect(brushEndListener).not.toBeCalled();

Expand All @@ -362,13 +367,6 @@ describe('Chart Store', () => {
expect(brushEndListener.mock.calls[1][1]).toBeCloseTo(0.9, 1);
});

test('can determine if brush is enabled', () => {
expect(store.isBrushEnabled()).toBe(true);

store.xScale = undefined;
expect(store.isBrushEnabled()).toBe(false);
});

test('can update parent dimensions', () => {
const computeChart = jest.fn(
(): void => {
Expand Down Expand Up @@ -552,6 +550,44 @@ describe('Chart Store', () => {
store.tooltipData.replace([tooltipValue]);
expect(store.isTooltipVisible.get()).toBe(true);
});

test('can disable brush based on scale and listener', () => {
store.xScale = undefined;
expect(store.isBrushEnabled()).toBe(false);
store.xScale = new ScaleContinuous([0, 100], [0, 100], ScaleType.Linear);
store.onBrushEndListener = undefined;
expect(store.isBrushEnabled()).toBe(false);
store.setOnBrushEndListener(() => ({}));
expect(store.isBrushEnabled()).toBe(true);
store.xScale = new ScaleBand([0, 100], [0, 100]);
expect(store.isBrushEnabled()).toBe(false);
});

test('can disable tooltip on brushing', () => {
const tooltipValue: TooltipValue = {
name: 'a',
value: 'a',
color: 'a',
isHighlighted: false,
isXValue: false,
};
store.xScale = new ScaleContinuous([0, 100], [0, 100], ScaleType.Linear);
store.cursorPosition.x = 1;
store.cursorPosition.x = 1;
store.tooltipType.set(TooltipType.Crosshairs);
store.tooltipData.replace([tooltipValue]);
store.onBrushStart();
expect(store.isBrushing.get()).toBe(true);
expect(store.isTooltipVisible.get()).toBe(false);

store.cursorPosition.x = 1;
store.cursorPosition.x = 1;
store.tooltipType.set(TooltipType.Crosshairs);
store.tooltipData.replace([tooltipValue]);
store.onBrushEnd({ x: 0, y: 0 }, { x: 1, y: 1 });
expect(store.isBrushing.get()).toBe(false);
expect(store.isTooltipVisible.get()).toBe(true);
});
test('handle click on chart', () => {
const geom1: IndexedGeometry = {
color: 'red',
Expand Down
40 changes: 26 additions & 14 deletions src/state/chart_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class ChartStore {
y: 0,
rotate: 0,
};
isBrushing = observable.box(false);
brushExtent: BrushExtent = {
minX: 0,
minY: 0,
Expand Down Expand Up @@ -385,6 +386,7 @@ export class ChartStore {

isTooltipVisible = computed(() => {
return (
!this.isBrushing.get() &&
this.tooltipType.get() !== TooltipType.None &&
this.cursorPosition.x > -1 &&
this.cursorPosition.y > -1 &&
Expand All @@ -393,6 +395,7 @@ export class ChartStore {
});
isCrosshairVisible = computed(() => {
return (
!this.isBrushing.get() &&
isCrosshairTooltipType(this.tooltipType.get()) &&
this.cursorPosition.x > -1 &&
this.cursorPosition.y > -1
Expand Down Expand Up @@ -521,6 +524,29 @@ export class ChartStore {
}
});

onBrushStart = action(() => {
if (!this.onBrushEndListener) {
return;
}
this.isBrushing.set(true);
});

onBrushEnd = action((start: Point, end: Point) => {
if (!this.onBrushEndListener) {
return;
}
this.isBrushing.set(false);
const minValue = start.x < end.x ? start.x : end.x;
const maxValue = start.x > end.x ? start.x : end.x;
if (maxValue === minValue) {
// if 0 size brush, avoid computing the value
return;
}
const min = this.xScale!.invert(minValue - this.chartDimensions.left);
const max = this.xScale!.invert(maxValue - this.chartDimensions.left);
this.onBrushEndListener(min, max);
});

handleChartClick() {
if (this.highlightedGeometries.length > 0 && this.onElementClickListener) {
this.onElementClickListener(this.highlightedGeometries.toJS());
Expand Down Expand Up @@ -579,20 +605,6 @@ export class ChartStore {
removeOnLegendItemMinusClickListener() {
this.onLegendItemMinusClickListener = undefined;
}
onBrushEnd(start: Point, end: Point) {
if (!this.onBrushEndListener) {
return;
}
const minValue = start.x < end.x ? start.x : end.x;
const maxValue = start.x > end.x ? start.x : end.x;
if (maxValue === minValue) {
// if 0 size brush, avoid computing the value
return;
}
const min = this.xScale!.invert(minValue - this.chartDimensions.left);
const max = this.xScale!.invert(maxValue - this.chartDimensions.left);
this.onBrushEndListener(min, max);
}

isBrushEnabled(): boolean {
if (!this.xScale) {
Expand Down

0 comments on commit 77d62f6

Please sign in to comment.