From 1ee7deebeb5ba978b9105953dd7e67215508f264 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Sat, 6 Jun 2020 17:31:01 -0400 Subject: [PATCH 1/7] first attempt to fix highlighting --- packages/react-vis/src/plot/highlight.js | 135 +++++++++++++++++++---- packages/website/storybook/misc-story.js | 23 +++- 2 files changed, 132 insertions(+), 26 deletions(-) diff --git a/packages/react-vis/src/plot/highlight.js b/packages/react-vis/src/plot/highlight.js index 05fdec519..9f5dd05ff 100644 --- a/packages/react-vis/src/plot/highlight.js +++ b/packages/react-vis/src/plot/highlight.js @@ -6,8 +6,10 @@ import {getAttributeScale} from 'utils/scales-utils'; import {getCombinedClassName} from 'utils/styling-utils'; function getLocs(evt) { - const xLoc = evt.type === 'touchstart' ? evt.pageX : evt.offsetX; - const yLoc = evt.type === 'touchstart' ? evt.pageY : evt.offsetY; + // const xLoc = evt.type === 'touchstart' ? evt.pageX : evt.offsetX; + // const yLoc = evt.type === 'touchstart' ? evt.pageY : evt.offsetY; + const xLoc = evt.offsetX ?? evt.pageX; + const yLoc = evt.offsetY ?? evt.pageY; return {xLoc, yLoc}; } @@ -20,7 +22,7 @@ class Highlight extends AbstractSeries { startLocY: 0, dragArea: null }; - + ref = React.createRef(); _getDrawArea(xLoc, yLoc) { const {startLocX, startLocY} = this.state; const { @@ -116,10 +118,69 @@ class Highlight extends AbstractSeries { return {}; } - startBrushing(e) { + _captureMouse = e => { + console.log('capture', e.type, e.target, e); + + document.addEventListener('mouseup', this.stopBrushing, {capture: true}); + document.addEventListener('mousemove', this.onBrush, {capture: true}); + // document.body.style['pointer-events'] = 'none'; + + if (this.ref.current) { + this.ref.current.addEventListener('mouseleave', this._mouseLeave, { + capture: true + }); + } + + e.preventDefault(); + e.stopPropagation(); + // this.startBrushing(e); + }; + + _releaseMouse = e => { + console.log('release', e.type, e.target, e); + + document.removeEventListener('mouseup', this.stopBrushing, {capture: true}); + document.removeEventListener('mousemove', this.onBrush, {capture: true}); + + if (this.ref.current) { + this.ref.current.removeEventListener('mouseleave', this._mouseLeave, { + capture: true + }); + } + // document.body.style['pointer-events'] = 'auto'; + e.stopPropagation(); + }; + + _mouseLeave = e => { + const {toElement} = e; + if (toElement === document.documentElement) { + this.stopBrushing(e); + return; + } + if (toElement === this.ref.current.parentNode.parentNode) { + this.stopBrushing(e); + return; + } + console.log('didnt really leave', toElement, this.ref.current.parentNode); + // this.ref.current. + }; + + startBrushing = e => { + // e.preventDefault(); + this._captureMouse(e); const {onBrushStart, onDragStart, drag} = this.props; const {dragArea} = this.state; - const {xLoc, yLoc} = getLocs(e.nativeEvent); + const {xLoc, yLoc} = getLocs(e); + console.log( + 'start', + xLoc, + yLoc, + e.type, + e.offsetX, + e.offsetY, + e.pageX, + e.pageY + ); const startArea = (dragging, resetDrag) => { const emptyBrush = { @@ -153,9 +214,23 @@ class Highlight extends AbstractSeries { onDragStart(e); } } - } + }; - stopBrushing() { + stopBrushing = e => { + if (e.toElement === document.documentElement) { + console.log('is document'); + // return; + } + console.log( + 'stop', + e.type, + e.target, + e.currentTarget, + e.toElement, + document, + e + ); + this._releaseMouse(e); const {brushing, dragging, brushArea} = this.state; // Quickly short-circuit if the user isn't brushing in our component if (!brushing && !dragging) { @@ -183,14 +258,18 @@ class Highlight extends AbstractSeries { if (drag && onDragEnd) { onDragEnd(!isNulled ? this._convertAreaToCoordinates(brushArea) : null); } - } + }; - onBrush(e) { + onBrush = e => { + e.preventDefault(); + e.stopPropagation(); const {onBrush, onDrag, drag} = this.props; const {brushing, dragging} = this.state; - const {xLoc, yLoc} = getLocs(e.nativeEvent); + const {xLoc, yLoc} = getLocs(e); + // console.log('brush', xLoc, yLoc); if (brushing) { const brushArea = this._getDrawArea(xLoc, yLoc); + // console.log('brush area', brushArea); this.setState({brushArea}); if (onBrush) { @@ -205,7 +284,7 @@ class Highlight extends AbstractSeries { onDrag(this._convertAreaToCoordinates(brushArea)); } } - } + }; render() { const { @@ -250,6 +329,7 @@ class Highlight extends AbstractSeries { className={getCombinedClassName(className, 'rv-highlight-container')} > this.startBrushing(e)} - onMouseMove={e => this.onBrush(e)} - onMouseUp={e => this.stopBrushing(e)} - onMouseLeave={e => this.stopBrushing(e)} + onMouseDownCapture={e => this.startBrushing(e.nativeEvent)} + // onMouseMoveCapture={e => this.onBrush(e)} + // onMouseUpCapture={e => this.stopBrushing(e)} + // onMouseLeave={e => { + // console.log( + // 'mouse leave', + // e.target, + // e.currentTarget, + // getLocs(e.nativeEvent) + // ); + // // this._releaseMouse(e); + // // this.stopBrushing(e); + // }} // preventDefault() so that mouse event emulation does not happen - onTouchEnd={e => { - e.preventDefault(); - this.stopBrushing(e); - }} - onTouchCancel={e => { - e.preventDefault(); - this.stopBrushing(e); - }} + // onTouchEnd={e => { + // e.preventDefault(); + // this.stopBrushing(e); + // }} + // onTouchCancel={e => { + // e.preventDefault(); + // this.stopBrushing(e); + // }} onContextMenu={e => e.preventDefault()} onContextMenuCapture={e => e.preventDefault()} /> diff --git a/packages/website/storybook/misc-story.js b/packages/website/storybook/misc-story.js index ba14d3c88..59fecdc3b 100644 --- a/packages/website/storybook/misc-story.js +++ b/packages/website/storybook/misc-story.js @@ -1,14 +1,14 @@ /* eslint-env node */ -import React from 'react'; +import React, {useState, useCallback} from 'react'; import {storiesOf} from '@storybook/react'; -import {withKnobs, boolean} from '@storybook/addon-knobs/react'; +import {withKnobs, boolean, button} from '@storybook/addon-knobs/react'; import {SimpleChartWrapper} from './storybook-utils'; import {generateLinearData} from './storybook-data'; -import {LineSeries, ContentClipPath} from 'react-vis'; +import {LineSeries, ContentClipPath, Highlight} from 'react-vis'; const data = generateLinearData({randomFactor: 10}); @@ -28,4 +28,21 @@ storiesOf('Misc', module) ); + }) + .addWithJSX('Highlight', () => { + const [zoom, setZoom] = useState(); + const onZoom = useCallback(area => { + console.log('zoom', area); + }, []); + + button('Reset Zoom', () => setZoom(null), 'Zoom'); + + const xDomain = zoom ? [zoom.left, zoom.right] : undefined; + + return ( + + + + + ); }); From 61a3c9d0234f1dfc00ef39c79836071929e2cde3 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Wed, 10 Jun 2020 17:30:31 -0400 Subject: [PATCH 2/7] Create ZoomHandler Due to the way that the Highlight component handles the mouse events, it is pretty much required that it is one of the last children of the XYPlot. Since it renders a transparent `rect` over the entire `XYPlot` it currently disables any mouse events for controls lower in the dom tree. If the `Highlight` component is moved higher in the dom tree, then the `onMouseLeave` event will fire when a lower component is hovered over. The ZoomHandler delegates the handling of the mouse events to the XYPlot itself. The XYPlot now holds a collection of `Event` objects that will be passed to the children, where they can subscribe to the ones that they are interested in. When the appropriate Event in the XYPlot is fired, it will execute the callbacks for all listeners. This approach does not cause the `XYPlot` to re-render, and only the listeners that perform an operation in their callback will be re-rendered. Also created a `useStateWithGet` that wraps a `useState` call and also provides a memoized `getState` method. Since the `getState` method is only created once for the component, it will not trigger re-renders when listed in the dependencies of `useEffect` / `useCallback`. This drastically cuts down the number of event handlers that are subscribed / unsubscribed. --- .eslintrc | 20 ++- packages/react-vis/package.json | 1 + packages/react-vis/src/index.js | 2 + packages/react-vis/src/plot/highlight.js | 135 +++------------ packages/react-vis/src/plot/xy-plot.js | 19 ++- packages/react-vis/src/plot/zoom-handler.js | 175 ++++++++++++++++++++ packages/react-vis/src/utils/events.js | 20 +++ packages/website/storybook/misc-story.js | 13 +- yarn.lock | 5 + 9 files changed, 265 insertions(+), 125 deletions(-) create mode 100644 packages/react-vis/src/plot/zoom-handler.js create mode 100644 packages/react-vis/src/utils/events.js diff --git a/.eslintrc b/.eslintrc index be727efd6..21d169a1e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -2,6 +2,7 @@ "extends": [ "eslint:recommended", "plugin:react/recommended", + "plugin:react-hooks/recommended", "plugin:jest/recommended", "prettier", "prettier/react" @@ -19,8 +20,8 @@ "**/dist/", "**/es/" ], - "settings":{ - "react":{ + "settings": { + "react": { "version": "detect" } }, @@ -31,12 +32,19 @@ }, "rules": { "consistent-return": 0, - "max-len": [1, 110, 4], - "max-params": ["error", 6], + "max-len": [ + 1, + 110, + 4 + ], + "max-params": [ + "error", + 6 + ], "object-curly-spacing": 0, "babel/object-curly-spacing": 2, - "jest/require-top-level-describe":"error", + "jest/require-top-level-describe": "error", "react/prop-types": "off", "prettier/prettier": "warn" } -} +} \ No newline at end of file diff --git a/packages/react-vis/package.json b/packages/react-vis/package.json index 8abe0a0a9..b1d3b1510 100644 --- a/packages/react-vis/package.json +++ b/packages/react-vis/package.json @@ -72,6 +72,7 @@ "enzyme-adapter-react-16": "^1.15.2", "enzyme-to-json": "^3.5.0", "eslint-plugin-jest": "^23.13.2", + "eslint-plugin-react-hooks": "^4.0.4", "jest": "^25.5.4", "jsdom": "^9.9.1", "node-sass": "^4.9.3", diff --git a/packages/react-vis/src/index.js b/packages/react-vis/src/index.js index 6a0b11d3e..ecceabd52 100644 --- a/packages/react-vis/src/index.js +++ b/packages/react-vis/src/index.js @@ -74,6 +74,8 @@ export Treemap from 'treemap'; export ContentClipPath from './plot/content-clip-path'; +export ZoomHandler from './plot/zoom-handler'; + export { makeHeightFlexible, makeVisFlexible, diff --git a/packages/react-vis/src/plot/highlight.js b/packages/react-vis/src/plot/highlight.js index 9f5dd05ff..05fdec519 100644 --- a/packages/react-vis/src/plot/highlight.js +++ b/packages/react-vis/src/plot/highlight.js @@ -6,10 +6,8 @@ import {getAttributeScale} from 'utils/scales-utils'; import {getCombinedClassName} from 'utils/styling-utils'; function getLocs(evt) { - // const xLoc = evt.type === 'touchstart' ? evt.pageX : evt.offsetX; - // const yLoc = evt.type === 'touchstart' ? evt.pageY : evt.offsetY; - const xLoc = evt.offsetX ?? evt.pageX; - const yLoc = evt.offsetY ?? evt.pageY; + const xLoc = evt.type === 'touchstart' ? evt.pageX : evt.offsetX; + const yLoc = evt.type === 'touchstart' ? evt.pageY : evt.offsetY; return {xLoc, yLoc}; } @@ -22,7 +20,7 @@ class Highlight extends AbstractSeries { startLocY: 0, dragArea: null }; - ref = React.createRef(); + _getDrawArea(xLoc, yLoc) { const {startLocX, startLocY} = this.state; const { @@ -118,69 +116,10 @@ class Highlight extends AbstractSeries { return {}; } - _captureMouse = e => { - console.log('capture', e.type, e.target, e); - - document.addEventListener('mouseup', this.stopBrushing, {capture: true}); - document.addEventListener('mousemove', this.onBrush, {capture: true}); - // document.body.style['pointer-events'] = 'none'; - - if (this.ref.current) { - this.ref.current.addEventListener('mouseleave', this._mouseLeave, { - capture: true - }); - } - - e.preventDefault(); - e.stopPropagation(); - // this.startBrushing(e); - }; - - _releaseMouse = e => { - console.log('release', e.type, e.target, e); - - document.removeEventListener('mouseup', this.stopBrushing, {capture: true}); - document.removeEventListener('mousemove', this.onBrush, {capture: true}); - - if (this.ref.current) { - this.ref.current.removeEventListener('mouseleave', this._mouseLeave, { - capture: true - }); - } - // document.body.style['pointer-events'] = 'auto'; - e.stopPropagation(); - }; - - _mouseLeave = e => { - const {toElement} = e; - if (toElement === document.documentElement) { - this.stopBrushing(e); - return; - } - if (toElement === this.ref.current.parentNode.parentNode) { - this.stopBrushing(e); - return; - } - console.log('didnt really leave', toElement, this.ref.current.parentNode); - // this.ref.current. - }; - - startBrushing = e => { - // e.preventDefault(); - this._captureMouse(e); + startBrushing(e) { const {onBrushStart, onDragStart, drag} = this.props; const {dragArea} = this.state; - const {xLoc, yLoc} = getLocs(e); - console.log( - 'start', - xLoc, - yLoc, - e.type, - e.offsetX, - e.offsetY, - e.pageX, - e.pageY - ); + const {xLoc, yLoc} = getLocs(e.nativeEvent); const startArea = (dragging, resetDrag) => { const emptyBrush = { @@ -214,23 +153,9 @@ class Highlight extends AbstractSeries { onDragStart(e); } } - }; + } - stopBrushing = e => { - if (e.toElement === document.documentElement) { - console.log('is document'); - // return; - } - console.log( - 'stop', - e.type, - e.target, - e.currentTarget, - e.toElement, - document, - e - ); - this._releaseMouse(e); + stopBrushing() { const {brushing, dragging, brushArea} = this.state; // Quickly short-circuit if the user isn't brushing in our component if (!brushing && !dragging) { @@ -258,18 +183,14 @@ class Highlight extends AbstractSeries { if (drag && onDragEnd) { onDragEnd(!isNulled ? this._convertAreaToCoordinates(brushArea) : null); } - }; + } - onBrush = e => { - e.preventDefault(); - e.stopPropagation(); + onBrush(e) { const {onBrush, onDrag, drag} = this.props; const {brushing, dragging} = this.state; - const {xLoc, yLoc} = getLocs(e); - // console.log('brush', xLoc, yLoc); + const {xLoc, yLoc} = getLocs(e.nativeEvent); if (brushing) { const brushArea = this._getDrawArea(xLoc, yLoc); - // console.log('brush area', brushArea); this.setState({brushArea}); if (onBrush) { @@ -284,7 +205,7 @@ class Highlight extends AbstractSeries { onDrag(this._convertAreaToCoordinates(brushArea)); } } - }; + } render() { const { @@ -329,7 +250,6 @@ class Highlight extends AbstractSeries { className={getCombinedClassName(className, 'rv-highlight-container')} > this.startBrushing(e.nativeEvent)} - // onMouseMoveCapture={e => this.onBrush(e)} - // onMouseUpCapture={e => this.stopBrushing(e)} - // onMouseLeave={e => { - // console.log( - // 'mouse leave', - // e.target, - // e.currentTarget, - // getLocs(e.nativeEvent) - // ); - // // this._releaseMouse(e); - // // this.stopBrushing(e); - // }} + onMouseDown={e => this.startBrushing(e)} + onMouseMove={e => this.onBrush(e)} + onMouseUp={e => this.stopBrushing(e)} + onMouseLeave={e => this.stopBrushing(e)} // preventDefault() so that mouse event emulation does not happen - // onTouchEnd={e => { - // e.preventDefault(); - // this.stopBrushing(e); - // }} - // onTouchCancel={e => { - // e.preventDefault(); - // this.stopBrushing(e); - // }} + onTouchEnd={e => { + e.preventDefault(); + this.stopBrushing(e); + }} + onTouchCancel={e => { + e.preventDefault(); + this.stopBrushing(e); + }} onContextMenu={e => e.preventDefault()} onContextMenuCapture={e => e.preventDefault()} /> diff --git a/packages/react-vis/src/plot/xy-plot.js b/packages/react-vis/src/plot/xy-plot.js index ccb503c5a..e8e87478a 100644 --- a/packages/react-vis/src/plot/xy-plot.js +++ b/packages/react-vis/src/plot/xy-plot.js @@ -50,6 +50,8 @@ import { import CanvasWrapper from './series/canvas-wrapper'; +import {Event} from '../utils/events'; + const ATTRIBUTES = [ 'x', 'y', @@ -140,7 +142,14 @@ class XYPlot extends React.Component { const data = getStackedData(children, stackBy); this.state = { scaleMixins: this._getScaleMixins(data, props), - data + data, + events: { + mouseMove: new Event('move'), + mouseDown: new Event('down'), + mouseUp: new Event('up'), + mouseLeave: new Event('leave'), + mouseEnter: new Event('enter') + } }; } @@ -222,7 +231,8 @@ class XYPlot extends React.Component { ...scaleMixins, ...child.props, ...XYPlotValues[index], - ...dataProps + ...dataProps, + events: this.state.events }); }); } @@ -348,6 +358,7 @@ class XYPlot extends React.Component { component.onParentMouseDown(event); } }); + this.state.events.mouseDown.fire(event); }; /** @@ -367,6 +378,7 @@ class XYPlot extends React.Component { component.onParentMouseEnter(event); } }); + this.state.events.mouseEnter.fire(event); }; /** @@ -386,6 +398,7 @@ class XYPlot extends React.Component { component.onParentMouseLeave(event); } }); + this.state.events.mouseLeave.fire(event); }; /** @@ -405,6 +418,7 @@ class XYPlot extends React.Component { component.onParentMouseMove(event); } }); + this.state.events.mouseMove.fire(event); }; /** @@ -424,6 +438,7 @@ class XYPlot extends React.Component { component.onParentMouseUp(event); } }); + this.state.events.mouseUp.fire(event); }; /** diff --git a/packages/react-vis/src/plot/zoom-handler.js b/packages/react-vis/src/plot/zoom-handler.js new file mode 100644 index 000000000..b6b4376cf --- /dev/null +++ b/packages/react-vis/src/plot/zoom-handler.js @@ -0,0 +1,175 @@ +import React, {useEffect, useState, useCallback, useRef} from 'react'; +import {getCombinedClassName} from '../utils/styling-utils'; +import {getAttributeScale} from '../utils/scales-utils'; + +const DEFAULT_STATE = { + brushing: false, + bounds: null, + startPosition: null +}; + +const useStateWithGet = initialState => { + const [state, setState] = useState(initialState); + const ref = useRef(); + ref.current = state; + const get = useCallback(() => ref.current, []); + return [state, setState, get]; +}; + +export default function ZoomHandler(props) { + const { + events: {mouseMove, mouseDown, mouseUp, mouseLeave}, + onZoom, + enableX = true, + enableY = true, + marginLeft = 0, + marginTop = 0, + innerWidth = 0, + innerHeight = 0 + } = props; + + const [state, setState, getState] = useStateWithGet(DEFAULT_STATE); + + const convertArea = useCallback( + area => { + const xScale = getAttributeScale(props, 'x'); + const yScale = getAttributeScale(props, 'y'); + + return { + bottom: yScale.invert(area.bottom), + left: xScale.invert(area.left - marginLeft), + right: xScale.invert(area.right - marginLeft), + top: yScale.invert(area.top) + }; + }, + [marginLeft, props] + ); + + const onMouseMove = useCallback( + e => { + const state = getState(); + if (!state.brushing) { + return; + } + e.stopPropagation(); + e.preventDefault(); + const position = getPosition(e); + + setState(state => { + const bounds = { + left: enableX + ? Math.min(position.x, state.startPosition.x) + : marginLeft, + top: enableY + ? Math.min(position.y, state.startPosition.y) + : marginTop, + right: enableX + ? Math.max(position.x, state.startPosition.x) + : innerWidth + marginLeft, + bottom: enableY + ? Math.max(position.y, state.startPosition.y) + : innerHeight + marginTop + }; + return { + ...state, + bounds + }; + }); + }, + [ + enableX, + enableY, + getState, + innerHeight, + innerWidth, + marginLeft, + marginTop, + setState + ] + ); + + const onMouseDown = useCallback( + e => { + e.stopPropagation(); + e.preventDefault(); + const {x, y} = getPosition(e); + + const bounds = {left: x, top: y, right: x, bottom: y}; + + setState(state => ({ + ...state, + brushing: true, + bounds, + startPosition: {x, y} + })); + }, + [setState] + ); + + const onMouseUp = useCallback( + e => { + const state = getState(); + + if (!state.brushing) { + return setState(DEFAULT_STATE); + } + + e.stopPropagation(); + e.preventDefault(); + + if ( + state.bounds.bottom - state.bounds.top > 5 && + state.bounds.right - state.bounds.left > 5 + ) { + onZoom && onZoom(convertArea(state.bounds)); + } + + setState(DEFAULT_STATE); + }, + [convertArea, getState, onZoom, setState] + ); + + const onMouseLeave = useCallback(() => { + const state = getState(); + if (state.brushing) { + setState(DEFAULT_STATE); + } + }, [getState, setState]); + + useEffect(() => mouseMove.subscribe(onMouseMove), [mouseMove, onMouseMove]); + useEffect(() => mouseDown.subscribe(onMouseDown), [mouseDown, onMouseDown]); + useEffect(() => mouseUp.subscribe(onMouseUp), [mouseUp, onMouseUp]); + useEffect(() => mouseLeave.subscribe(onMouseLeave), [ + mouseLeave, + onMouseLeave + ]); + + if (!state.brushing) { + return null; + } + + const {bounds} = state; + const {opacity, color, className} = props; + + return ( + + + + ); +} +ZoomHandler.requiresSVG = true; + +function getPosition(evt) { + const x = evt.nativeEvent.offsetX ?? evt.nativeEvent.pageX; + const y = evt.nativeEvent.offsetY ?? evt.nativeEvent.pageY; + return {x, y}; +} diff --git a/packages/react-vis/src/utils/events.js b/packages/react-vis/src/utils/events.js new file mode 100644 index 000000000..ceb5bc12f --- /dev/null +++ b/packages/react-vis/src/utils/events.js @@ -0,0 +1,20 @@ +export class Event { + subscribers = []; + + constructor(name) { + this.name = name; + } + + fire(...args) { + this.subscribers.forEach(cb => cb(...args)); + } + + subscribe(callback) { + this.subscribers.push(callback); + return () => this.unsubscribe(callback); + } + + unsubscribe(callback) { + this.subscribers = this.subscribers.filter(x => x !== callback); + } +} diff --git a/packages/website/storybook/misc-story.js b/packages/website/storybook/misc-story.js index 59fecdc3b..18aa79702 100644 --- a/packages/website/storybook/misc-story.js +++ b/packages/website/storybook/misc-story.js @@ -8,10 +8,12 @@ import {withKnobs, boolean, button} from '@storybook/addon-knobs/react'; import {SimpleChartWrapper} from './storybook-utils'; import {generateLinearData} from './storybook-data'; -import {LineSeries, ContentClipPath, Highlight} from 'react-vis'; +import {LineSeries, MarkSeries, ContentClipPath, ZoomHandler} from 'react-vis'; const data = generateLinearData({randomFactor: 10}); +const highlightData = generateLinearData({}); + storiesOf('Misc', module) .addDecorator(withKnobs) .addWithJSX('Clip Content', () => { @@ -33,16 +35,17 @@ storiesOf('Misc', module) const [zoom, setZoom] = useState(); const onZoom = useCallback(area => { console.log('zoom', area); + setZoom(area); }, []); button('Reset Zoom', () => setZoom(null), 'Zoom'); const xDomain = zoom ? [zoom.left, zoom.right] : undefined; - + const yDomain = zoom ? [zoom.bottom, zoom.top] : undefined; return ( - - - + + + ); }); diff --git a/yarn.lock b/yarn.lock index 52b54f597..500c15f32 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6603,6 +6603,11 @@ eslint-plugin-prettier@^3.1.3: dependencies: prettier-linter-helpers "^1.0.0" +eslint-plugin-react-hooks@^4.0.4: + version "4.0.4" + resolved "https://registry.yarnpkg.com/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-4.0.4.tgz#aed33b4254a41b045818cacb047b81e6df27fa58" + integrity sha1-rtM7QlSkGwRYGMrLBHuB5t8n+lg= + eslint-plugin-react@^6.7.1: version "6.10.3" resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-6.10.3.tgz#c5435beb06774e12c7db2f6abaddcbf900cd3f78" From e4bb21ce3ca3dd4dd0f0dc41f956bd44769402d7 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Wed, 10 Jun 2020 19:22:32 -0400 Subject: [PATCH 3/7] removed 'useStateWithGet' --- packages/react-vis/src/plot/zoom-handler.js | 68 +++++++++------------ 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/packages/react-vis/src/plot/zoom-handler.js b/packages/react-vis/src/plot/zoom-handler.js index b6b4376cf..2d5c4869b 100644 --- a/packages/react-vis/src/plot/zoom-handler.js +++ b/packages/react-vis/src/plot/zoom-handler.js @@ -8,14 +8,6 @@ const DEFAULT_STATE = { startPosition: null }; -const useStateWithGet = initialState => { - const [state, setState] = useState(initialState); - const ref = useRef(); - ref.current = state; - const get = useCallback(() => ref.current, []); - return [state, setState, get]; -}; - export default function ZoomHandler(props) { const { events: {mouseMove, mouseDown, mouseUp, mouseLeave}, @@ -28,7 +20,15 @@ export default function ZoomHandler(props) { innerHeight = 0 } = props; - const [state, setState, getState] = useStateWithGet(DEFAULT_STATE); + const [state, setState] = useState(DEFAULT_STATE); + const stateRef = useRef(); + // The 'state' is being assigned to the 'ref' so that the `useCallback`s can + // reference the value without directly depending on it. + // This is important for performance reasons, as directly depending on the state, + // will cause the event handlers to be added and removed for each move of the mouse. + // The lifecycle of the callbacks isn't affected by the value of the 'state', so + // there is no harm in using the `stateRef` to get the latest value of the `state` + stateRef.current = state; const convertArea = useCallback( area => { @@ -47,7 +47,8 @@ export default function ZoomHandler(props) { const onMouseMove = useCallback( e => { - const state = getState(); + // Get the current value of 'state' + const state = stateRef.current; if (!state.brushing) { return; } @@ -76,39 +77,28 @@ export default function ZoomHandler(props) { }; }); }, - [ - enableX, - enableY, - getState, - innerHeight, - innerWidth, - marginLeft, - marginTop, - setState - ] + [enableX, enableY, innerHeight, innerWidth, marginLeft, marginTop] ); - const onMouseDown = useCallback( - e => { - e.stopPropagation(); - e.preventDefault(); - const {x, y} = getPosition(e); + const onMouseDown = useCallback(e => { + e.stopPropagation(); + e.preventDefault(); + const {x, y} = getPosition(e); - const bounds = {left: x, top: y, right: x, bottom: y}; + const bounds = {left: x, top: y, right: x, bottom: y}; - setState(state => ({ - ...state, - brushing: true, - bounds, - startPosition: {x, y} - })); - }, - [setState] - ); + setState(state => ({ + ...state, + brushing: true, + bounds, + startPosition: {x, y} + })); + }, []); const onMouseUp = useCallback( e => { - const state = getState(); + // Get the current value of 'state' + const state = stateRef.current; if (!state.brushing) { return setState(DEFAULT_STATE); @@ -126,15 +116,15 @@ export default function ZoomHandler(props) { setState(DEFAULT_STATE); }, - [convertArea, getState, onZoom, setState] + [convertArea, onZoom] ); const onMouseLeave = useCallback(() => { - const state = getState(); + const state = stateRef.current; if (state.brushing) { setState(DEFAULT_STATE); } - }, [getState, setState]); + }, []); useEffect(() => mouseMove.subscribe(onMouseMove), [mouseMove, onMouseMove]); useEffect(() => mouseDown.subscribe(onMouseDown), [mouseDown, onMouseDown]); From e51407b7053985ae6672992644aa7a07b416db67 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Thu, 11 Jun 2020 10:26:10 -0400 Subject: [PATCH 4/7] add tests --- .../react-vis/tests/plot/zoom-handler.test.js | 91 +++++++++++++++++++ packages/react-vis/tests/utils/events.test.js | 27 ++++++ packages/website/storybook/misc-story.js | 3 +- packages/website/storybook/storybook-utils.js | 1 + 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 packages/react-vis/tests/plot/zoom-handler.test.js create mode 100644 packages/react-vis/tests/utils/events.test.js diff --git a/packages/react-vis/tests/plot/zoom-handler.test.js b/packages/react-vis/tests/plot/zoom-handler.test.js new file mode 100644 index 000000000..671d3b34d --- /dev/null +++ b/packages/react-vis/tests/plot/zoom-handler.test.js @@ -0,0 +1,91 @@ +import React from 'react'; + +import {mount} from 'enzyme'; +import ZoomHandler from '../../src/plot/zoom-handler'; +import XYPlot from '../../src/plot/xy-plot'; + +describe('zoom-handler', () => { + it('should zoom', () => { + const onZoom = jest.fn(); + const wrapper = mount( + + + + ); + + const svg = wrapper.find('svg'); + svg.simulate('mousedown', mouseEvent(100, 100)); + svg.simulate('mouseMove', mouseEvent(150, 150)); + svg.simulate('mouseUp', mouseEvent(150, 150)); + + expect(onZoom).toBeCalledWith({ + left: 4.8, + top: 12, + right: 8.8, + bottom: 8 + }); + }); + + it('should render selection', () => { + const onZoom = jest.fn(); + const wrapper = mount( + + + + ); + + const svg = wrapper.find('svg'); + svg.simulate('mousedown', mouseEvent(100, 100)); + svg.simulate('mouseMove', mouseEvent(150, 150)); + + const selection = wrapper.find('rect'); + + expect(selection.props()).toMatchObject({ + x: 100, + y: 100, + width: 50, + height: 50 + }); + }); + + it('should clear selection if plot mouseleave', () => { + const onZoom = jest.fn(); + const wrapper = mount( + + + + ); + + const svg = wrapper.find('svg'); + svg.simulate('mousedown', mouseEvent(100, 100)); + svg.simulate('mousemove', mouseEvent(150, 150)); + + expect(wrapper.find('rect')).toHaveLength(1); + + svg.simulate('mouseleave'); + expect(wrapper.find('rect')).toHaveLength(0); + expect(onZoom).not.toBeCalled(); + }); +}); + +function mouseEvent(x, y) { + return {nativeEvent: {offsetX: x, offsetY: y}}; +} diff --git a/packages/react-vis/tests/utils/events.test.js b/packages/react-vis/tests/utils/events.test.js new file mode 100644 index 000000000..cc3616405 --- /dev/null +++ b/packages/react-vis/tests/utils/events.test.js @@ -0,0 +1,27 @@ +import {Event} from '../../src/utils/events'; + +describe('events', () => { + it('should subscribe', () => { + const event = new Event(); + const handler = jest.fn(); + event.subscribe(handler); + expect(event.subscribers).toHaveLength(1); + + event.fire('foo'); + expect(handler).toHaveBeenCalledWith('foo'); + }); + + it('should unsubscribe', () => { + const event = new Event(); + const handler = jest.fn(); + const unsubscribe = event.subscribe(handler); + expect(event.subscribers).toHaveLength(1); + + unsubscribe(); + expect(event.subscribers).toHaveLength(0); + + event.fire('foo'); + + expect(handler).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/website/storybook/misc-story.js b/packages/website/storybook/misc-story.js index 18aa79702..177794204 100644 --- a/packages/website/storybook/misc-story.js +++ b/packages/website/storybook/misc-story.js @@ -31,10 +31,9 @@ storiesOf('Misc', module) ); }) - .addWithJSX('Highlight', () => { + .addWithJSX('Zoom', () => { const [zoom, setZoom] = useState(); const onZoom = useCallback(area => { - console.log('zoom', area); setZoom(area); }, []); diff --git a/packages/website/storybook/storybook-utils.js b/packages/website/storybook/storybook-utils.js index bfd996591..a5a8a3a34 100644 --- a/packages/website/storybook/storybook-utils.js +++ b/packages/website/storybook/storybook-utils.js @@ -54,6 +54,7 @@ export function SimpleChartWrapper(props) { xType={props.xType} yDomain={props.yDomain || [0, 20]} stackBy={props.stackBy} + animation={props.animation} > {props.noXAxis ? null From c7332a0e60c9951bf6cedea3c1e0ee173310bb17 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Sat, 6 Jun 2020 19:40:34 -0400 Subject: [PATCH 5/7] Allow using Storybook for local development. Storybook is an easy to use UI to enable testing out features. This allows storybook to pick up changes from react-vis/src so that it can be used while developing components. --- packages/website/.babelrc | 1 + packages/website/.storybook/main.js | 28 ++++++++++++++++++++++++++++ packages/website/webpack.config.js | 3 ++- 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 packages/website/.babelrc create mode 100644 packages/website/.storybook/main.js diff --git a/packages/website/.babelrc b/packages/website/.babelrc new file mode 100644 index 000000000..9e26dfeeb --- /dev/null +++ b/packages/website/.babelrc @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/packages/website/.storybook/main.js b/packages/website/.storybook/main.js new file mode 100644 index 000000000..a8fbec58d --- /dev/null +++ b/packages/website/.storybook/main.js @@ -0,0 +1,28 @@ +/* eslint-env node */ +const {join} = require('path'); +const isProd = process.env.NODE_ENV === 'production'; + +// This custom webpack config allows storybook to use the source files from 'react-vis'. +// Changes can be made and instantly be reflected in storybook to allow quick development +module.exports = { + webpackFinal: config => { + if (isProd) { return config;} + + const reactVisPath = join(__dirname, '../../react-vis/src') + + // Add an alias to 'react-vis' so that it looks in its src directory. + config.resolve.alias['react-vis'] =reactVisPath + config.resolve.modules.push(reactVisPath) + + const jsRule = config.module.rules.find(rule => rule.test.test('test.js')); + // Add the react-vis/src folder to the list of files to compile + jsRule.include.push(reactVisPath) + + const babelLoader = jsRule.use.find(x => x.loader === 'babel-loader'); + if (babelLoader) { + babelLoader.options.rootMode = 'upward' + } + + return config; + } +} diff --git a/packages/website/webpack.config.js b/packages/website/webpack.config.js index 833884df2..6c3420dcb 100644 --- a/packages/website/webpack.config.js +++ b/packages/website/webpack.config.js @@ -16,7 +16,8 @@ module.exports = { }, resolve: { alias: { - react: resolve(__dirname, './node_modules/react') + react: resolve(__dirname, './node_modules/react'), + 'react-vis': resolve('../react-vis/src') } } }; From 10da92efa27f7d84f73a2b9fd4d651eaf3428813 Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Fri, 12 Jun 2020 23:03:33 -0400 Subject: [PATCH 6/7] Added Window Added a Window component that renders a moveable window primarily used for visualizing a moveable viewport. Added a storybook that showcases this. --- packages/react-vis/src/index.js | 3 +- .../plot/{zoom-handler.js => selection.js} | 112 +++++++------ packages/react-vis/src/plot/window.js | 157 ++++++++++++++++++ packages/website/.storybook/storybook.css | 11 +- packages/website/storybook/misc-story.js | 51 +++++- packages/website/storybook/storybook-utils.js | 83 ++++----- 6 files changed, 319 insertions(+), 98 deletions(-) rename packages/react-vis/src/plot/{zoom-handler.js => selection.js} (57%) create mode 100644 packages/react-vis/src/plot/window.js diff --git a/packages/react-vis/src/index.js b/packages/react-vis/src/index.js index ecceabd52..2c4c549af 100644 --- a/packages/react-vis/src/index.js +++ b/packages/react-vis/src/index.js @@ -74,7 +74,8 @@ export Treemap from 'treemap'; export ContentClipPath from './plot/content-clip-path'; -export ZoomHandler from './plot/zoom-handler'; +export Selection from './plot/selection'; +export Window from './plot/window'; export { makeHeightFlexible, diff --git a/packages/react-vis/src/plot/zoom-handler.js b/packages/react-vis/src/plot/selection.js similarity index 57% rename from packages/react-vis/src/plot/zoom-handler.js rename to packages/react-vis/src/plot/selection.js index 2d5c4869b..456db7cb8 100644 --- a/packages/react-vis/src/plot/zoom-handler.js +++ b/packages/react-vis/src/plot/selection.js @@ -1,5 +1,4 @@ import React, {useEffect, useState, useCallback, useRef} from 'react'; -import {getCombinedClassName} from '../utils/styling-utils'; import {getAttributeScale} from '../utils/scales-utils'; const DEFAULT_STATE = { @@ -8,26 +7,29 @@ const DEFAULT_STATE = { startPosition: null }; -export default function ZoomHandler(props) { +export default function Selection(props) { const { events: {mouseMove, mouseDown, mouseUp, mouseLeave}, - onZoom, + onSelecting, + onSelected, enableX = true, enableY = true, marginLeft = 0, marginTop = 0, innerWidth = 0, - innerHeight = 0 + innerHeight = 0, + xDomain, + yDomain } = props; const [state, setState] = useState(DEFAULT_STATE); - const stateRef = useRef(); // The 'state' is being assigned to the 'ref' so that the `useCallback`s can // reference the value without directly depending on it. // This is important for performance reasons, as directly depending on the state, // will cause the event handlers to be added and removed for each move of the mouse. // The lifecycle of the callbacks isn't affected by the value of the 'state', so // there is no harm in using the `stateRef` to get the latest value of the `state` + const stateRef = useRef(); stateRef.current = state; const convertArea = useCallback( @@ -35,14 +37,16 @@ export default function ZoomHandler(props) { const xScale = getAttributeScale(props, 'x'); const yScale = getAttributeScale(props, 'y'); + // If the axis isn't enabled, then use the domain to ensure + // that the entire space is selected. return { - bottom: yScale.invert(area.bottom), - left: xScale.invert(area.left - marginLeft), - right: xScale.invert(area.right - marginLeft), - top: yScale.invert(area.top) + left: enableX ? xScale.invert(area.left - marginLeft) : xDomain[0], + top: enableY ? yScale.invert(area.top - marginTop) : yDomain[1], + right: enableX ? xScale.invert(area.right - marginLeft) : yDomain[1], + bottom: enableY ? yScale.invert(area.bottom - marginTop) : yDomain[0] }; }, - [marginLeft, props] + [enableX, enableY, marginLeft, marginTop, props, xDomain, yDomain] ); const onMouseMove = useCallback( @@ -56,28 +60,36 @@ export default function ZoomHandler(props) { e.preventDefault(); const position = getPosition(e); - setState(state => { - const bounds = { - left: enableX - ? Math.min(position.x, state.startPosition.x) - : marginLeft, - top: enableY - ? Math.min(position.y, state.startPosition.y) - : marginTop, - right: enableX - ? Math.max(position.x, state.startPosition.x) - : innerWidth + marginLeft, - bottom: enableY - ? Math.max(position.y, state.startPosition.y) - : innerHeight + marginTop - }; - return { - ...state, - bounds - }; + const bounds = { + left: enableX + ? Math.min(position.x, state.startPosition.x) + : marginLeft, + top: enableY ? Math.min(position.y, state.startPosition.y) : marginTop, + right: enableX + ? Math.max(position.x, state.startPosition.x) + : innerWidth + marginLeft, + bottom: enableY + ? Math.max(position.y, state.startPosition.y) + : innerHeight + marginTop + }; + + onSelecting && onSelecting(convertArea(bounds)); + + setState({ + ...state, + bounds }); }, - [enableX, enableY, innerHeight, innerWidth, marginLeft, marginTop] + [ + convertArea, + enableX, + enableY, + innerHeight, + innerWidth, + marginLeft, + marginTop, + onSelecting + ] ); const onMouseDown = useCallback(e => { @@ -111,12 +123,14 @@ export default function ZoomHandler(props) { state.bounds.bottom - state.bounds.top > 5 && state.bounds.right - state.bounds.left > 5 ) { - onZoom && onZoom(convertArea(state.bounds)); + onSelected && onSelected(convertArea(state.bounds)); + } else { + onSelected && onSelected(null); } setState(DEFAULT_STATE); }, - [convertArea, onZoom] + [convertArea, onSelected] ); const onMouseLeave = useCallback(() => { @@ -139,27 +153,27 @@ export default function ZoomHandler(props) { } const {bounds} = state; - const {opacity, color, className} = props; + const {opacity = 0.2, color, style} = props; return ( - - - + ); } -ZoomHandler.requiresSVG = true; +Selection.requiresSVG = true; -function getPosition(evt) { - const x = evt.nativeEvent.offsetX ?? evt.nativeEvent.pageX; - const y = evt.nativeEvent.offsetY ?? evt.nativeEvent.pageY; +function getPosition(event) { + event = event.nativeEvent ?? event; + const x = event.type === 'touchstart' ? event.pageX : event.offsetX; + const y = event.type === 'touchstart' ? event.pageY : event.offsetY; return {x, y}; } diff --git a/packages/react-vis/src/plot/window.js b/packages/react-vis/src/plot/window.js new file mode 100644 index 000000000..ff5838165 --- /dev/null +++ b/packages/react-vis/src/plot/window.js @@ -0,0 +1,157 @@ +import React, {useMemo, useCallback, useRef, useState, useEffect} from 'react'; +import {getAttributeScale} from '../utils/scales-utils'; + +const DEFAULT_STATE = { + dragging: false, + startPosition: null, + offset: null, + + bounds: null +}; + +export default function Window(props) { + const { + yDomain, + xDomain, + left = xDomain[0], + top = yDomain[1], + right = xDomain[1], + bottom = yDomain[0], + onMoving, + onMoveComplete, + enableX = true, + enableY = true, + events: {mouseMove, mouseLeave} + } = props; + + const xScale = useMemo(() => getAttributeScale(props, 'x'), [props]); + const yScale = useMemo(() => getAttributeScale(props, 'y'), [props]); + + const [state, setState] = useState(DEFAULT_STATE); + const stateRef = useRef(); + stateRef.current = state; + + const pixelBounds = useMemo(() => { + return { + x: xScale(left) + (state.offset?.x ?? 0), + y: yScale(top) + (state.offset?.y ?? 0), + width: xScale(right) - xScale(left), + height: yScale(bottom) - yScale(top) + }; + }, [ + bottom, + left, + right, + state.offset?.x, + state.offset?.y, + top, + xScale, + yScale + ]); + + const onMouseDown = useCallback(e => { + e.stopPropagation(); + e.preventDefault(); + + setState({ + dragging: true, + startPosition: getPosition(e), + offset: {x: 0, y: 0} + }); + }, []); + + const onMouseMove = useCallback( + e => { + const {dragging, startPosition} = stateRef.current; + if (!dragging) { + return; + } + e.stopPropagation(); + e.preventDefault(); + + const position = getPosition(e); + const pixelOffset = { + x: enableX ? position.x - startPosition.x : 0, + y: enableY ? position.y - startPosition.y : 0 + }; + + const bounds = { + left: xScale.invert(xScale(left) + pixelOffset.x), + top: yScale.invert(yScale(top) + pixelOffset.y), + right: xScale.invert(xScale(right) + pixelOffset.x), + bottom: yScale.invert(yScale(bottom) + pixelOffset.y) + }; + + onMoving && onMoving(bounds); + + setState(state => ({ + ...state, + offset: pixelOffset, + bounds + })); + }, + [bottom, enableX, enableY, left, onMoving, right, top, xScale, yScale] + ); + + const onMouseUp = useCallback( + e => { + const {dragging} = stateRef.current; + if (!dragging) { + return; + } + e.stopPropagation(); + e.preventDefault(); + + const state = stateRef.current; + + setState(DEFAULT_STATE); + onMoveComplete && onMoveComplete(state.bounds); + }, + [onMoveComplete] + ); + + const onPlotMouseLeave = useCallback(() => { + const state = stateRef.current; + if (state.dragging) { + setState(DEFAULT_STATE); + } + }, []); + + useEffect(() => mouseMove.subscribe(onMouseMove), [mouseMove, onMouseMove]); + useEffect(() => mouseLeave.subscribe(onPlotMouseLeave), [ + mouseLeave, + onPlotMouseLeave + ]); + + if ( + [pixelBounds.x, pixelBounds.y, pixelBounds.width, pixelBounds.height].some( + isNaN + ) + ) { + return null; + } + + const {color, opacity = 0.2, style, marginLeft, marginTop} = props; + + return ( + + ); +} +Window.requiresSVG = true; + +function getPosition(event) { + event = event.nativeEvent ?? event; + const x = event.type === 'touchstart' ? event.pageX : event.offsetX; + const y = event.type === 'touchstart' ? event.pageY : event.offsetY; + return {x, y}; +} diff --git a/packages/website/.storybook/storybook.css b/packages/website/.storybook/storybook.css index 7cc411c30..f75cb06fc 100644 --- a/packages/website/.storybook/storybook.css +++ b/packages/website/.storybook/storybook.css @@ -1,3 +1,8 @@ -html, body, #root { - height: 100%; -} +html, +body, +#root { + height: 100%; + box-sizing: border-box; + position: relative; + margin: 0 +} \ No newline at end of file diff --git a/packages/website/storybook/misc-story.js b/packages/website/storybook/misc-story.js index 177794204..6ab65e811 100644 --- a/packages/website/storybook/misc-story.js +++ b/packages/website/storybook/misc-story.js @@ -8,12 +8,21 @@ import {withKnobs, boolean, button} from '@storybook/addon-knobs/react'; import {SimpleChartWrapper} from './storybook-utils'; import {generateLinearData} from './storybook-data'; -import {LineSeries, MarkSeries, ContentClipPath, ZoomHandler} from 'react-vis'; +import {LineSeries, ContentClipPath, Selection, Window} from 'react-vis'; const data = generateLinearData({randomFactor: 10}); const highlightData = generateLinearData({}); - +const yDomainHighlightData = [ + highlightData.reduce( + (min, cur) => Math.floor(Math.min(min, cur.y)), + Number.MAX_SAFE_INTEGER + ), + highlightData.reduce( + (max, cur) => Math.ceil(Math.max(max, cur.y)), + Number.MIN_SAFE_INTEGER + ) +]; storiesOf('Misc', module) .addDecorator(withKnobs) .addWithJSX('Clip Content', () => { @@ -33,18 +42,46 @@ storiesOf('Misc', module) }) .addWithJSX('Zoom', () => { const [zoom, setZoom] = useState(); - const onZoom = useCallback(area => { + const onSelected = useCallback(area => { setZoom(area); }, []); button('Reset Zoom', () => setZoom(null), 'Zoom'); const xDomain = zoom ? [zoom.left, zoom.right] : undefined; - const yDomain = zoom ? [zoom.bottom, zoom.top] : undefined; + return ( - - - + + + ); + }) + .addWithJSX('Window', () => { + const [zoom, setZoom] = useState({left: 5, right: 10}); + const onMoveComplete = useCallback(area => { + setZoom(area); + }, []); + + const xDomain = [zoom.left, zoom.right]; + + return ( +
+
+ + + + +
+
+ + + + +
+
+ ); }); diff --git a/packages/website/storybook/storybook-utils.js b/packages/website/storybook/storybook-utils.js index a5a8a3a34..b2372df2b 100644 --- a/packages/website/storybook/storybook-utils.js +++ b/packages/website/storybook/storybook-utils.js @@ -39,44 +39,51 @@ export const LINEAR_PALETTE = ['#EF5D28', '#FF9833']; export function SimpleChartWrapper(props) { return ( - {({height, width}) => ( - - {props.noXAxis - ? null - : boolean('X Axis', true, 'General chart options') && } - {props.noYAxis - ? null - : boolean('Y Axis', true, 'General chart options') && } - {props.noVerticalGridLines - ? null - : boolean('vertical gridlines', true, 'General chart options') && ( - - )} - {props.noHorizontalGridLines - ? null - : boolean( - 'horizontal gridlines', - true, - 'General chart options' - ) && } - {props.children} - - )} + {({height, width}) => { + if (!width || !height) { + return null; + } + return ( + + {props.noXAxis + ? null + : boolean('X Axis', true, 'General chart options') && } + {props.noYAxis + ? null + : boolean('Y Axis', true, 'General chart options') && } + {props.noVerticalGridLines + ? null + : boolean( + 'vertical gridlines', + true, + 'General chart options' + ) && } + {props.noHorizontalGridLines + ? null + : boolean( + 'horizontal gridlines', + true, + 'General chart options' + ) && } + {props.children} + + ); + }} ); } From 2932243aa8d87c4a0c71fd2e9f394b34f83508be Mon Sep 17 00:00:00 2001 From: Chris Thomas Date: Fri, 12 Jun 2020 23:10:49 -0400 Subject: [PATCH 7/7] fix test --- .../react-vis/tests/plot/zoom-handler.test.js | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/react-vis/tests/plot/zoom-handler.test.js b/packages/react-vis/tests/plot/zoom-handler.test.js index 671d3b34d..6b9adef14 100644 --- a/packages/react-vis/tests/plot/zoom-handler.test.js +++ b/packages/react-vis/tests/plot/zoom-handler.test.js @@ -1,12 +1,12 @@ import React from 'react'; import {mount} from 'enzyme'; -import ZoomHandler from '../../src/plot/zoom-handler'; +import Selection from '../../src/plot/selection'; import XYPlot from '../../src/plot/xy-plot'; describe('zoom-handler', () => { it('should zoom', () => { - const onZoom = jest.fn(); + const onSelected = jest.fn(); const wrapper = mount( { xDomain={[0, 20]} yDomain={[0, 20]} > - + ); const svg = wrapper.find('svg'); - svg.simulate('mousedown', mouseEvent(100, 100)); - svg.simulate('mouseMove', mouseEvent(150, 150)); - svg.simulate('mouseUp', mouseEvent(150, 150)); + svg.simulate('mousedown', mouseEvent(100, 110)); + svg.simulate('mouseMove', mouseEvent(150, 160)); + svg.simulate('mouseUp', mouseEvent(150, 160)); - expect(onZoom).toBeCalledWith({ + expect(onSelected).toBeCalledWith({ left: 4.8, top: 12, right: 8.8, @@ -33,7 +33,7 @@ describe('zoom-handler', () => { }); it('should render selection', () => { - const onZoom = jest.fn(); + const onSelected = jest.fn(); const wrapper = mount( { xDomain={[0, 20]} yDomain={[0, 20]} > - + ); @@ -61,7 +61,7 @@ describe('zoom-handler', () => { }); it('should clear selection if plot mouseleave', () => { - const onZoom = jest.fn(); + const onSelected = jest.fn(); const wrapper = mount( { xDomain={[0, 20]} yDomain={[0, 20]} > - + ); @@ -82,7 +82,7 @@ describe('zoom-handler', () => { svg.simulate('mouseleave'); expect(wrapper.find('rect')).toHaveLength(0); - expect(onZoom).not.toBeCalled(); + expect(onSelected).not.toBeCalled(); }); });