From 83ae1f873cd233593dfae3a15a4ad75952654c15 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 20 Aug 2019 19:46:09 +0200 Subject: [PATCH 1/4] fix: decuple brush cursor from chart rendering This commit remove the unnecessary style recalculation caused by chancing the cursor style on the document body from the chart_state. This commit also decouple the chart container from the chart renderer, removing multiple rendering caused by the mouse leaving the brush area --- .playground/playgroud.tsx | 2 +- src/chart_types/xy_chart/store/chart_state.ts | 9 -- src/components/_container.scss | 19 ++++ src/components/chart.tsx | 6 +- src/components/chart_resizer.tsx | 15 +-- .../react_canvas/chart_container.tsx | 45 +++++++++ .../react_canvas/reactive_chart.tsx | 94 +++++++------------ 7 files changed, 101 insertions(+), 89 deletions(-) create mode 100644 src/components/react_canvas/chart_container.tsx diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index bf77de95a6..4572220b38 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -59,7 +59,7 @@ function renderChart( return (
- + { @@ -552,8 +545,6 @@ export class ChartStore { this.highlightedGeometries.clear(); this.tooltipData.clear(); - document.body.style.cursor = 'default'; - if (this.onCursorUpdateListener && this.isActiveChart.get()) { this.onCursorUpdateListener(); } diff --git a/src/components/_container.scss b/src/components/_container.scss index a1dd5b6068..214ee859c0 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -17,3 +17,22 @@ .echChart--isBrushEnabled { cursor: crosshair; } + +.echChartCursorContainer { + position: absolute; + top: 0; + bottom: 0; + right: 0; + left: 0; + box-sizing: border-box; +} + +.echChartResizer { + z-index: -10000000; + position: absolute; + bottom: 0; + top: 0; + left: 0; + right: 0; + box-sizing: border-box; +} \ No newline at end of file diff --git a/src/components/chart.tsx b/src/components/chart.tsx index a2bafc6d6a..ac0b629486 100644 --- a/src/components/chart.tsx +++ b/src/components/chart.tsx @@ -10,7 +10,7 @@ import { Crosshair } from './crosshair'; import { Highlighter } from './highlighter'; import { Legend } from './legend/legend'; import { LegendButton } from './legend/legend_button'; -import { ReactiveChart as ReactChart } from './react_canvas/reactive_chart'; +import { ChartContainer } from './react_canvas/chart_container'; import { Tooltips } from './tooltips'; import { CursorEvent } from '../specs/settings'; import { ChartSize, getChartSize } from '../utils/chart_size'; @@ -75,8 +75,8 @@ export class Chart extends React.Component { {// TODO reenable when SVG rendered is aligned with canvas one - renderer === 'svg' && } - {renderer === 'canvas' && } + renderer === 'svg' && } + {renderer === 'canvas' && } diff --git a/src/components/chart_resizer.tsx b/src/components/chart_resizer.tsx index 1a6ea13b82..10cd605997 100644 --- a/src/components/chart_resizer.tsx +++ b/src/components/chart_resizer.tsx @@ -35,20 +35,7 @@ class Resizer extends React.Component { }; render() { - return ( -
- ); + return
; } private handleResize = (entries: ResizeObserverEntry[]) => { diff --git a/src/components/react_canvas/chart_container.tsx b/src/components/react_canvas/chart_container.tsx new file mode 100644 index 0000000000..0313139a73 --- /dev/null +++ b/src/components/react_canvas/chart_container.tsx @@ -0,0 +1,45 @@ +import React from 'react'; +import classNames from 'classnames'; +import { inject, observer } from 'mobx-react'; +import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; +import { ReactiveChart } from './reactive_chart'; +interface ReactiveChartProps { + chartStore?: ChartStore; // FIX until we find a better way on ts mobx +} + +class ChartContainerComponent extends React.Component { + static displayName = 'ChartContainer'; + + render() { + const { initialized } = this.props.chartStore!; + if (!initialized.get()) { + return null; + } + const className = classNames('echChartCursorContainer', { + 'echChart--isBrushEnabled': this.props.chartStore!.isCrosshairCursorVisible.get(), + }); + const { setCursorPosition } = this.props.chartStore!; + + return ( +
{ + setCursorPosition(offsetX, offsetY); + }} + onMouseLeave={() => { + setCursorPosition(-1, -1); + }} + onMouseUp={() => { + if (this.props.chartStore!.isBrushing.get()) { + return; + } + this.props.chartStore!.handleChartClick(); + }} + > + +
+ ); + } +} + +export const ChartContainer = inject('chartStore')(observer(ChartContainerComponent)); diff --git a/src/components/react_canvas/reactive_chart.tsx b/src/components/react_canvas/reactive_chart.tsx index a347d2da96..cc8d68e5f1 100644 --- a/src/components/react_canvas/reactive_chart.tsx +++ b/src/components/react_canvas/reactive_chart.tsx @@ -1,4 +1,3 @@ -import classNames from 'classnames'; import { inject, observer } from 'mobx-react'; import React from 'react'; import { Layer, Rect, Stage } from 'react-konva'; @@ -352,7 +351,6 @@ class Chart extends React.Component { chartRotation, chartTransform, debug, - setCursorPosition, isChartEmpty, legendCollapsed, legendPosition, @@ -406,76 +404,48 @@ class Chart extends React.Component { clipHeight: chartDimensions.height, }; - const className = classNames({ - 'echChart--isBrushEnabled': this.props.chartStore!.isCrosshairCursorVisible.get(), - }); - return ( -
{ - setCursorPosition(offsetX, offsetY); + width: '100%', + height: '100%', }} - onMouseLeave={() => { - setCursorPosition(-1, -1); - }} - onMouseUp={() => { - if (this.props.chartStore!.isBrushing.get()) { - return; - } - this.props.chartStore!.handleChartClick(); - }} - className={className} + {...brushProps} > - + {this.renderGrids()} + + + - - {this.renderGrids()} - - - - {this.sortAndRenderElements()} - + {this.sortAndRenderElements()} + + + {debug && this.renderDebugChartBorders()} + + {isBrushEnabled && ( - {debug && this.renderDebugChartBorders()} + {this.renderBrushTool()} - {isBrushEnabled && ( - - {this.renderBrushTool()} - - )} + )} - - {this.renderAxes()} - + + {this.renderAxes()} + - - {this.renderBarValues()} - - -
+ + {this.renderBarValues()} + + ); } From 3c65d031f82966813816db8f48c2e5d75105dcfe Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 21 Aug 2019 13:19:29 +0200 Subject: [PATCH 2/4] fix: recover pointer when hovering on geometries --- .../xy_chart/store/chart_state.test.ts | 46 +++++++++++++++++-- src/chart_types/xy_chart/store/chart_state.ts | 13 +++--- src/components/_container.scss | 4 -- .../react_canvas/chart_container.tsx | 10 ++-- 4 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/chart_types/xy_chart/store/chart_state.test.ts b/src/chart_types/xy_chart/store/chart_state.test.ts index de7e3f7d87..ef8e7d8768 100644 --- a/src/chart_types/xy_chart/store/chart_state.test.ts +++ b/src/chart_types/xy_chart/store/chart_state.test.ts @@ -908,21 +908,61 @@ describe('Chart Store', () => { store.cursorPosition.x = -1; store.cursorPosition.y = -1; store.onBrushEndListener = brushEndListener; - expect(store.isCrosshairCursorVisible.get()).toBe(false); + expect(store.chartCursor.get()).toBe('default'); }); test('when cursor is within chart bounds and brush enabled', () => { store.cursorPosition.x = 10; store.cursorPosition.y = 10; store.onBrushEndListener = brushEndListener; - expect(store.isCrosshairCursorVisible.get()).toBe(true); + expect(store.chartCursor.get()).toBe('crosshair'); }); test('when cursor is within chart bounds and brush disabled', () => { store.cursorPosition.x = 10; store.cursorPosition.y = 10; store.onBrushEndListener = undefined; - expect(store.isCrosshairCursorVisible.get()).toBe(false); + expect(store.chartCursor.get()).toBe('default'); + }); + test('when cursor is within chart bounds and brush enabled but over one geom', () => { + store.cursorPosition.x = 10; + store.cursorPosition.y = 10; + store.onBrushEndListener = brushEndListener; + const geom1: IndexedGeometry = { + color: 'red', + geometryId: { + specId: getSpecId('specId1'), + seriesKey: [2], + }, + value: { + x: 0, + y: 1, + accessor: 'y1', + }, + x: 0, + y: 0, + width: 0, + height: 0, + seriesStyle: { + rect: { + opacity: 1, + }, + rectBorder: { + strokeWidth: 1, + visible: false, + }, + displayValue: { + fill: 'black', + fontFamily: '', + fontSize: 2, + offsetX: 0, + offsetY: 0, + padding: 2, + }, + }, + }; + store.highlightedGeometries.replace([geom1]); + expect(store.chartCursor.get()).toBe('pointer'); }); }); test('should set tooltip type to follow when single value x scale', () => { diff --git a/src/chart_types/xy_chart/store/chart_state.ts b/src/chart_types/xy_chart/store/chart_state.ts index 9a6c3a63b0..d102740bc2 100644 --- a/src/chart_types/xy_chart/store/chart_state.ts +++ b/src/chart_types/xy_chart/store/chart_state.ts @@ -241,17 +241,16 @@ export class ChartStore { this.computeChart(); }); - /** - * determine if crosshair cursor should be visible based on cursor position and brush enablement - */ - isCrosshairCursorVisible = computed(() => { + chartCursor = computed(() => { const { x: xPos, y: yPos } = this.cursorPosition; if (yPos < 0 || xPos < 0) { - return false; + return 'default'; } - - return this.isBrushEnabled(); + if (this.highlightedGeometries.length > 0) { + return 'pointer'; + } + return this.isBrushEnabled() ? 'crosshair' : 'default'; }); /** diff --git a/src/components/_container.scss b/src/components/_container.scss index 214ee859c0..50b6435bfb 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -14,10 +14,6 @@ } } -.echChart--isBrushEnabled { - cursor: crosshair; -} - .echChartCursorContainer { position: absolute; top: 0; diff --git a/src/components/react_canvas/chart_container.tsx b/src/components/react_canvas/chart_container.tsx index 0313139a73..5b2046b46b 100644 --- a/src/components/react_canvas/chart_container.tsx +++ b/src/components/react_canvas/chart_container.tsx @@ -1,5 +1,4 @@ import React from 'react'; -import classNames from 'classnames'; import { inject, observer } from 'mobx-react'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; import { ReactiveChart } from './reactive_chart'; @@ -15,14 +14,13 @@ class ChartContainerComponent extends React.Component { if (!initialized.get()) { return null; } - const className = classNames('echChartCursorContainer', { - 'echChart--isBrushEnabled': this.props.chartStore!.isCrosshairCursorVisible.get(), - }); const { setCursorPosition } = this.props.chartStore!; - return (
{ setCursorPosition(offsetX, offsetY); }} From bc67539eddb6c84cd070a6a3e57511e020669c52 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 21 Aug 2019 14:34:54 +0200 Subject: [PATCH 3/4] fix: show pointer only with onElementClick or onElementOver listeners --- .playground/playgroud.tsx | 2 +- src/chart_types/xy_chart/store/chart_state.test.ts | 2 ++ src/chart_types/xy_chart/store/chart_state.ts | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index 4572220b38..bf77de95a6 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -59,7 +59,7 @@ function renderChart( return (
- + { }, }; store.highlightedGeometries.replace([geom1]); + expect(store.chartCursor.get()).toBe('crosshair'); + store.onElementClickListener = jest.fn(); expect(store.chartCursor.get()).toBe('pointer'); }); }); diff --git a/src/chart_types/xy_chart/store/chart_state.ts b/src/chart_types/xy_chart/store/chart_state.ts index d102740bc2..578f82be32 100644 --- a/src/chart_types/xy_chart/store/chart_state.ts +++ b/src/chart_types/xy_chart/store/chart_state.ts @@ -247,7 +247,7 @@ export class ChartStore { if (yPos < 0 || xPos < 0) { return 'default'; } - if (this.highlightedGeometries.length > 0) { + if (this.highlightedGeometries.length > 0 && (this.onElementClickListener || this.onElementOverListener)) { return 'pointer'; } return this.isBrushEnabled() ? 'crosshair' : 'default'; From 1b30f40c7d711de107b4201b80bf660dad760aa6 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 21 Aug 2019 19:49:51 +0200 Subject: [PATCH 4/4] fix: typo on css merge --- src/components/_container.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/_container.scss b/src/components/_container.scss index b92ab54ede..4a01d60487 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -14,6 +14,7 @@ &--column { flex-direction: column; } +} .echChartCursorContainer { position: absolute;