From 62ee988cc631060cdf005d3605cf6eec5f7258b0 Mon Sep 17 00:00:00 2001
From: Stas Vilchik <stas.vilchik@sonarsource.com>
Date: Mon, 17 Dec 2018 14:57:06 +0100
Subject: [PATCH] improve code quality

---
 .../main/js/components/charts/BubbleChart.tsx | 139 ++++++++----------
 .../charts/__tests__/BubbleChart-test.tsx     |  16 +-
 2 files changed, 73 insertions(+), 82 deletions(-)

diff --git a/server/sonar-web/src/main/js/components/charts/BubbleChart.tsx b/server/sonar-web/src/main/js/components/charts/BubbleChart.tsx
index 49a50d4ffcc..d0383d31496 100644
--- a/server/sonar-web/src/main/js/components/charts/BubbleChart.tsx
+++ b/server/sonar-web/src/main/js/components/charts/BubbleChart.tsx
@@ -23,8 +23,8 @@ import { AutoSizer } from 'react-virtualized/dist/commonjs/AutoSizer';
 import { Link } from 'react-router';
 import { min, max } from 'd3-array';
 import { scaleLinear, ScaleLinear } from 'd3-scale';
-import { zoom, zoomIdentity } from 'd3-zoom';
-import { event, select } from 'd3-selection';
+import { zoom, zoomIdentity, ZoomBehavior } from 'd3-zoom';
+import { event, select, Selection } from 'd3-selection';
 import { sortBy, uniq } from 'lodash';
 import Tooltip from '../controls/Tooltip';
 import { translate } from '../../helpers/l10n';
@@ -33,50 +33,6 @@ import './BubbleChart.css';
 
 const TICKS_COUNT = 5;
 
-interface BubbleProps<T> {
-  color?: string;
-  link?: string | Location;
-  onClick?: (ref?: T) => void;
-  data?: T;
-  r: number;
-  scale: number;
-  tooltip?: string | React.ReactNode;
-  x: number;
-  y: number;
-}
-
-export class Bubble<T> extends React.PureComponent<BubbleProps<T>> {
-  handleClick = (event: React.MouseEvent<SVGCircleElement>) => {
-    if (this.props.onClick) {
-      event.stopPropagation();
-      event.preventDefault();
-      this.props.onClick(this.props.data);
-    }
-  };
-
-  render() {
-    let circle = (
-      <circle
-        className="bubble-chart-bubble"
-        onClick={this.props.onClick ? this.handleClick : undefined}
-        r={this.props.r}
-        style={{ fill: this.props.color, stroke: this.props.color }}
-        transform={`translate(${this.props.x}, ${this.props.y}) scale(${this.props.scale})`}
-      />
-    );
-
-    if (this.props.link && !this.props.onClick) {
-      circle = <Link to={this.props.link}>{circle}</Link>;
-    }
-
-    return (
-      <Tooltip overlay={this.props.tooltip || undefined}>
-        <g>{circle}</g>
-      </Tooltip>
-    );
-  }
-}
-
 interface BubbleItem<T> {
   color?: string;
   key?: string;
@@ -93,8 +49,8 @@ interface Props<T> {
   displayXTicks?: boolean;
   displayYGrid?: boolean;
   displayYTicks?: boolean;
-  formatXTick?: (tick: number) => string;
-  formatYTick?: (tick: number) => string;
+  formatXTick: (tick: number) => string;
+  formatYTick: (tick: number) => string;
   height: number;
   items: BubbleItem<T>[];
   onBubbleClick?: (ref?: T) => void;
@@ -111,18 +67,19 @@ interface State {
 
 type Scale = ScaleLinear<number, number>;
 
-export default class BubbleChart<T> extends React.Component<Props<T>, State> {
-  node: SVGSVGElement | null = null;
-  selection: any = null;
-  transform: any = null;
-  zoom: any = null;
+export default class BubbleChart<T> extends React.PureComponent<Props<T>, State> {
+  node?: Element;
+  selection?: Selection<Element, {}, null, undefined>;
+  zoom?: ZoomBehavior<Element, {}>;
 
   static defaultProps = {
     displayXGrid: true,
     displayXTicks: true,
     displayYGrid: true,
     displayYTicks: true,
-    padding: [0, 0, 0, 0],
+    formatXTick: (d: number) => String(d),
+    formatYTick: (d: number) => String(d),
+    padding: [10, 10, 10, 10],
     sizeRange: [5, 45]
   };
 
@@ -161,21 +118,11 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
   resetZoom = (event: React.MouseEvent<Link>) => {
     event.stopPropagation();
     event.preventDefault();
-    select(this.node).call(this.zoom.transform, zoomIdentity);
+    if (this.zoom && this.node) {
+      select(this.node).call(this.zoom.transform, zoomIdentity);
+    }
   };
 
-  get formatXTick() {
-    return this.props.formatXTick || ((d: number) => String(d));
-  }
-
-  get formatYTick() {
-    return this.props.formatYTick || ((d: number) => String(d));
-  }
-
-  get padding() {
-    return this.props.padding || [10, 10, 10, 10];
-  }
-
   getXRange(xScale: Scale, sizeScale: Scale, availableWidth: number) {
     const minX = min(this.props.items, d => xScale(d.x) - sizeScale(d.size)) || 0;
     const maxX = max(this.props.items, d => xScale(d.x) + sizeScale(d.size)) || 0;
@@ -262,7 +209,7 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
     const ticks = xTicks.map((tick, index) => {
       const x = xScale(tick) * transform.k + transform.x;
       const y = yScale.range()[0];
-      const innerText = this.formatXTick(tick);
+      const innerText = this.props.formatXTick(tick);
       return x > 0 ? (
         <text className="bubble-chart-tick" dy="1.5em" key={index} x={x} y={y}>
           {innerText}
@@ -282,7 +229,7 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
     const ticks = yTicks.map((tick, index) => {
       const x = xScale.range()[0];
       const y = yScale(tick) * transform.k + transform.y;
-      const innerText = this.formatYTick(tick);
+      const innerText = this.props.formatYTick(tick);
       return y > 0 && y < this.props.height - 80 ? (
         <text
           className="bubble-chart-tick bubble-chart-tick-y"
@@ -301,8 +248,8 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
 
   renderChart = (width: number) => {
     const { transform } = this.state;
-    const availableWidth = width - this.padding[1] - this.padding[3];
-    const availableHeight = this.props.height - this.padding[0] - this.padding[2];
+    const availableWidth = width - this.props.padding[1] - this.props.padding[3];
+    const availableHeight = this.props.height - this.props.padding[0] - this.props.padding[2];
 
     const xScale = scaleLinear()
       .domain(this.props.xDomain || [0, max(this.props.items, d => d.x) || 0])
@@ -339,8 +286,8 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
       );
     });
 
-    const xTicks = this.getTicks(xScale, this.formatXTick);
-    const yTicks = this.getTicks(yScale, this.formatYTick);
+    const xTicks = this.getTicks(xScale, this.props.formatXTick);
+    const yTicks = this.getTicks(yScale, this.props.formatYTick);
 
     return (
       <svg
@@ -352,14 +299,14 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
           <clipPath id="graph-region">
             <rect
               // Extend clip by 2 pixels: one for clipRect border, and one for Bubble borders
-              height={this.props.height - this.padding[0] - this.padding[2] + 4}
+              height={this.props.height - this.props.padding[0] - this.props.padding[2] + 4}
               width={width + 4}
               x={-2}
               y={-2}
             />
           </clipPath>
         </defs>
-        <g transform={`translate(${this.padding[3]}, ${this.padding[0]})`}>
+        <g transform={`translate(${this.props.padding[3]}, ${this.props.padding[0]})`}>
           <g clipPath="url(#graph-region)">
             {this.renderXGrid(xTicks, xScale, yScale)}
             {this.renderYGrid(yTicks, xScale, yScale)}
@@ -392,3 +339,45 @@ export default class BubbleChart<T> extends React.Component<Props<T>, State> {
     );
   }
 }
+
+interface BubbleProps<T> {
+  color?: string;
+  link?: string | Location;
+  onClick?: (ref?: T) => void;
+  data?: T;
+  r: number;
+  scale: number;
+  tooltip?: string | React.ReactNode;
+  x: number;
+  y: number;
+}
+
+function Bubble<T>(props: BubbleProps<T>) {
+  const handleClick = (event: React.MouseEvent<SVGCircleElement>) => {
+    if (props.onClick) {
+      event.stopPropagation();
+      event.preventDefault();
+      props.onClick(props.data);
+    }
+  };
+
+  let circle = (
+    <circle
+      className="bubble-chart-bubble"
+      onClick={props.onClick ? handleClick : undefined}
+      r={props.r}
+      style={{ fill: props.color, stroke: props.color }}
+      transform={`translate(${props.x}, ${props.y}) scale(${props.scale})`}
+    />
+  );
+
+  if (props.link && !props.onClick) {
+    circle = <Link to={props.link}>{circle}</Link>;
+  }
+
+  return (
+    <Tooltip overlay={props.tooltip || undefined}>
+      <g>{circle}</g>
+    </Tooltip>
+  );
+}
diff --git a/server/sonar-web/src/main/js/components/charts/__tests__/BubbleChart-test.tsx b/server/sonar-web/src/main/js/components/charts/__tests__/BubbleChart-test.tsx
index df39a250e12..d85783c2fbb 100644
--- a/server/sonar-web/src/main/js/components/charts/__tests__/BubbleChart-test.tsx
+++ b/server/sonar-web/src/main/js/components/charts/__tests__/BubbleChart-test.tsx
@@ -20,7 +20,7 @@
 import * as React from 'react';
 import { mount } from 'enzyme';
 import { AutoSizerProps } from 'react-virtualized';
-import BubbleChart, { Bubble } from '../BubbleChart';
+import BubbleChart from '../BubbleChart';
 
 jest.mock('react-virtualized/dist/commonjs/AutoSizer', () => ({
   AutoSizer: ({ children }: AutoSizerProps) => children({ width: 100, height: NaN })
@@ -28,19 +28,21 @@ jest.mock('react-virtualized/dist/commonjs/AutoSizer', () => ({
 
 it('should display bubbles', () => {
   const items = [{ x: 1, y: 10, size: 7 }, { x: 2, y: 30, size: 5 }];
-  const chart = mount(<BubbleChart height={100} items={items} />);
-  chart.find(Bubble).forEach(bubble => expect(bubble).toMatchSnapshot());
+  const chart = mount(<BubbleChart height={100} items={items} padding={[0, 0, 0, 0]} />);
+  chart.find('Bubble').forEach(bubble => expect(bubble).toMatchSnapshot());
 });
 
 it('should render bubble links', () => {
   const items = [{ x: 1, y: 10, size: 7, link: 'foo' }, { x: 2, y: 30, size: 5, link: 'bar' }];
-  const chart = mount(<BubbleChart height={100} items={items} />);
-  chart.find(Bubble).forEach(bubble => expect(bubble).toMatchSnapshot());
+  const chart = mount(<BubbleChart height={100} items={items} padding={[0, 0, 0, 0]} />);
+  chart.find('Bubble').forEach(bubble => expect(bubble).toMatchSnapshot());
 });
 
 it('should render bubbles with click handlers', () => {
   const onClick = jest.fn();
   const items = [{ x: 1, y: 10, size: 7, data: 'foo' }, { x: 2, y: 30, size: 5, data: 'bar' }];
-  const chart = mount(<BubbleChart height={100} items={items} onBubbleClick={onClick} />);
-  chart.find(Bubble).forEach(bubble => expect(bubble).toMatchSnapshot());
+  const chart = mount(
+    <BubbleChart height={100} items={items} onBubbleClick={onClick} padding={[0, 0, 0, 0]} />
+  );
+  chart.find('Bubble').forEach(bubble => expect(bubble).toMatchSnapshot());
 });
-- 
2.39.5