fix after review DevExpress#2

Krijovnick · Nov 15, 2018 · a4b4c8a · a4b4c8a
1 parent 45c2c48
commit a4b4c8a
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 28 deletions.
diff --git a/packages/dx-chart-core/src/plugins/tooltip/computeds.js b/packages/dx-chart-core/src/plugins/tooltip/computeds.js
@@ -1,5 +1,18 @@
+import { processPointerMove } from '../../utils/hover-state';
+
 export const getParameters = (series, target) => {
   const currentSeries = series.find(({ name }) => target.series === name);
   const item = currentSeries.points.find(point => point.index === target.point);
   return { element: currentSeries.getTargetElement(item), text: `${item.value}` };
 };
+
+export const processHandleTooltip = (targets, currentTarget, onTargetItemChange) => {
+  if (targets.length && targets[0].point !== undefined) {
+    const target = processPointerMove(targets, currentTarget, onTargetItemChange);
+    if (target !== undefined) {
+      return { target };
+    }
+    return null;
+  }
+  return { target: null };
+};
diff --git a/packages/dx-chart-core/src/plugins/tooltip/computeds.test.js b/packages/dx-chart-core/src/plugins/tooltip/computeds.test.js
@@ -1,6 +1,14 @@
 import {
   getParameters,
+  processHandleTooltip,
 } from './computeds';
+import {
+  processPointerMove,
+} from '../../utils/hover-state';
+
+jest.mock('../../utils/hover-state', () => ({
+  processPointerMove: jest.fn().mockReturnValue('test-target'),
+}));
 
 describe('#getParameters', () => {
   const createSeries = name => ({
@@ -9,7 +17,36 @@ describe('#getParameters', () => {
     getTargetElement: jest.fn(() => 'parameters'),
   });
   const series = [createSeries('s1'), createSeries('s2'), createSeries('s3')];
+
   it('should return text and element', () => {
     expect(getParameters(series, { series: 's2', point: 1 })).toEqual({ element: 'parameters', text: '20' });
   });
 });
+
+describe('#processHandleTooltip', () => {
+  afterEach(jest.clearAllMocks);
+
+  it('should return target', () => {
+    expect(processHandleTooltip([{ series: 'test-series', point: 'test-point' }], 'currentTarget', 'mockFunction')).toEqual({
+      target: 'test-target',
+    });
+    expect(processPointerMove).toBeCalledWith([{ series: 'test-series', point: 'test-point' }], 'currentTarget', 'mockFunction');
+  });
+
+  it('should return null target, targets contain only series field', () => {
+    expect(processHandleTooltip([{ series: 'test-series' }], 'currentTarget', 'mockFunction')).toEqual({
+      target: null,
+    });
+  });
+
+  it('should return null target, targets are empty', () => {
+    expect(processHandleTooltip([], 'currentTarget', 'mockFunction')).toEqual({
+      target: null,
+    });
+  });
+
+  it('should return null, target the same', () => {
+    processPointerMove.mockReturnValue(undefined);
+    expect(processHandleTooltip([{ series: 'test-series', point: 'test-point' }], 'currentTarget', 'mockFunction')).toEqual(null);
+  });
+});
diff --git a/packages/dx-react-chart-bootstrap4/src/templates/tooltip/overlay.jsx b/packages/dx-react-chart-bootstrap4/src/templates/tooltip/overlay.jsx
@@ -3,11 +3,11 @@ import * as PropTypes from 'prop-types';
 import { Popover, PopoverBody } from 'reactstrap';
 
 export const Overlay = ({
-  visible, children, target, ...restProps
+  children, target, ...restProps
 }) => (
   <Popover
     placement="top"
-    isOpen={visible}
+    isOpen
     target={target}
     {...restProps}
   >
@@ -19,6 +19,5 @@ export const Overlay = ({
 
 Overlay.propTypes = {
   children: PropTypes.node.isRequired,
-  visible: PropTypes.bool.isRequired,
   target: PropTypes.func.isRequired,
 };
diff --git a/packages/dx-react-chart-bootstrap4/src/templates/tooltip/overlay.test.jsx b/packages/dx-react-chart-bootstrap4/src/templates/tooltip/overlay.test.jsx
@@ -5,7 +5,6 @@ import { Overlay } from './overlay';
 describe('Overlay', () => {
   const defaultProps = {
     target: () => {},
-    visible: true,
   };
 
   it('should render Popover', () => {

diff --git a/packages/dx-react-chart-material-ui/src/templates/tooltip/overlay.jsx b/packages/dx-react-chart-material-ui/src/templates/tooltip/overlay.jsx
@@ -3,10 +3,10 @@ import * as PropTypes from 'prop-types';
 import Popover from '@material-ui/core/Popover';
 
 export const Overlay = ({
-  visible, children, target, ...restProps
+  children, target, ...restProps
 }) => (
   <Popover
-    open={visible}
+    open
     anchorEl={target}
     anchorOrigin={{ vertical: 'center', horizontal: 'center' }}
     transformOrigin={{ vertical: 'bottom', horizontal: 'center' }}
@@ -18,6 +18,5 @@ export const Overlay = ({
 
 Overlay.propTypes = {
   children: PropTypes.node.isRequired,
-  visible: PropTypes.bool.isRequired,
   target: PropTypes.func.isRequired,
 };
diff --git a/packages/dx-react-chart-material-ui/src/templates/tooltip/overlay.test.jsx b/packages/dx-react-chart-material-ui/src/templates/tooltip/overlay.test.jsx
@@ -5,7 +5,6 @@ import { Overlay } from './overlay';
 describe('Overlay', () => {
   const defaultProps = {
     target: () => {},
-    visible: true,
   };
 
   it('should render Popover', () => {

diff --git a/packages/dx-react-chart/src/plugins/tooltip.jsx b/packages/dx-react-chart/src/plugins/tooltip.jsx
@@ -8,7 +8,7 @@ import {
   TemplatePlaceholder,
   withComponents,
 } from '@devexpress/dx-react-core';
-import { getParameters, processPointerMove } from '@devexpress/dx-chart-core';
+import { getParameters, processHandleTooltip } from '@devexpress/dx-chart-core';
 import { Target } from '../templates/tooltip/target';
 
 class RawTooltip extends React.PureComponent {
@@ -17,6 +17,7 @@ class RawTooltip extends React.PureComponent {
     this.state = {
       target: props.targetItem || props.defaultTargetItem,
     };
+    this.createTargetElement = this.createTargetElement.bind(this);
     this.getTargetElement = this.getTargetElement.bind(this);
     const handlePointerMove = this.handlePointerMove.bind(this);
     this.getPointerMoveHandlers = ({ pointerMoveHandlers }) => [
@@ -32,17 +33,15 @@ class RawTooltip extends React.PureComponent {
     return this.targetElement;
   }
 
+  createTargetElement(ref) {
+    this.targetElement = ref;
+  }
+
   handlePointerMove({ targets }) {
-    this.setState(({ target: currentTarget }, { onTargetItemChange }) => {
-      const target = processPointerMove(targets, currentTarget, onTargetItemChange);
-      if (target !== undefined) {
-        if (target && target.point === undefined) {
-          return { target: null };
-        }
-        return { target };
-      }
-      return null;
-    });
+    this.setState((
+      { target: currentTarget },
+      { onTargetItemChange },
+    ) => processHandleTooltip(targets, currentTarget, onTargetItemChange));
   }
 
   render() {
@@ -70,12 +69,11 @@ class RawTooltip extends React.PureComponent {
                 <React.Fragment>
                   <TargetComponent
                     {...element}
-                    componentRef={(ref) => { this.targetElement = ref; }}
+                    componentRef={this.createTargetElement}
                   />
                   <OverlayComponent
                     key={`${target.series}${target.point}`}
                     target={this.getTargetElement}
-                    visible
                   >
                     <ContentComponent text={text} targetItem={target} />
                   </OverlayComponent>

diff --git a/packages/dx-react-chart/src/plugins/tooltip.test.jsx b/packages/dx-react-chart/src/plugins/tooltip.test.jsx
@@ -2,12 +2,12 @@ import * as React from 'react';
 import { mount } from 'enzyme';
 import { PluginHost } from '@devexpress/dx-react-core';
 import { pluginDepsToComponents, getComputedState } from '@devexpress/dx-react-core/test-utils';
-import { processPointerMove } from '@devexpress/dx-chart-core';
+import { processHandleTooltip } from '@devexpress/dx-chart-core';
 import { Tooltip } from './tooltip';
 
 jest.mock('@devexpress/dx-chart-core', () => ({
   getParameters: jest.fn().mockReturnValue({ element: { x: 10, y: 20 }, text: 'tooltip-text' }),
-  processPointerMove: jest.fn().mockReturnValue('test-target'),
+  processHandleTooltip: jest.fn().mockReturnValue('test-target'),
 }));
 
 const TargetComponent = () => null;
@@ -74,12 +74,11 @@ describe('Tooltip', () => {
 
         <Tooltip {...defaultProps} targetItem={{ series: '1', point: 4 }} />
       </PluginHost>));
-    const { children, target, visible } = tree.find(OverlayComponent).props();
+    const { children, target } = tree.find(OverlayComponent).props();
 
     expect(children)
       .toBeTruthy();
     expect(target).toEqual(expect.any(Function));
-    expect(visible).toBeTruthy();
   });
 
   it('should render contentComponent', () => {
@@ -101,7 +100,6 @@ describe('Tooltip', () => {
         <Tooltip {...defaultProps} defaultTargetItem={{ series: '2', point: 3 }} targetItem={{ series: '1', point: 4 }} />
       </PluginHost>));
 
-    expect(tree.find(OverlayComponent).props().visible).toBeTruthy();
     expect(tree.find(ContentComponent).props()).toEqual({ text: 'tooltip-text', targetItem: { series: '1', point: 4 } });
   });
 
@@ -113,7 +111,6 @@ describe('Tooltip', () => {
         <Tooltip {...defaultProps} defaultTargetItem={{ series: '2', point: 3 }} />
       </PluginHost>));
 
-    expect(tree.find(OverlayComponent).props().visible).toBeTruthy();
     expect(tree.find(ContentComponent).props()).toEqual({ text: 'tooltip-text', targetItem: { series: '2', point: 3 } });
   });
 
@@ -128,6 +125,6 @@ describe('Tooltip', () => {
     ));
     getComputedState(tree).pointerMoveHandlers[1]({ targets: 'test-targets' });
 
-    expect(processPointerMove).toBeCalledWith('test-targets', { series: '1', point: 2 }, mock);
+    expect(processHandleTooltip).toBeCalledWith('test-targets', { series: '1', point: 2 }, mock);
   });
 });