Add ability to copy node data from graph (#312)

* * Refactor KV Table copy icon into standalone component

Signed-off-by: Everett Ross <reverett@uber.com>

* Adhere to prettier rules

Signed-off-by: Everett Ross <reverett@uber.com>

* Add <CopyIcon /> to drawNode and OpNode

Signed-off-by: Everett Ross <reverett@uber.com>

* Improve variable names for splitting CopyIcon into own file

Signed-off-by: Everett Ross <reverett@uber.com>

* Clean up CopyIcon

Signed-off-by: Everett Ross <reverett@uber.com>

* Fix KeyValuesTable test coverage

Signed-off-by: Everett Ross <reverett@uber.com>

packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css

@@ -72,6 +72,13 @@ limitations under the License.
 
 .DiffNode--labelCell {
   padding: 0.3rem 0.5rem 0.3rem 0.75rem;
+  display: flex;
+  justify-content: space-between;
+}
+
+.DiffNode--popover .DiffNode--copyIcon,
+.DiffNode:not(:hover) .DiffNode--copyIcon {
+  display: none;
 }
 
 /* Tweak the popover aesthetics - unfortunate but necessary */

packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js

@@ -18,6 +18,8 @@ import * as React from 'react';
 import { Popover } from 'antd';
 import cx from 'classnames';
 
+import CopyIcon from '../../common/CopyIcon';
+
 import type { PVertex } from '../../../model/trace-dag/types';
 
 import './drawNode.css';
@@ -57,6 +59,11 @@ class DiffNode extends React.PureComponent<Props> {
             </td>
             <td className={`DiffNode--labelCell ${className}`}>
               <strong>{service}</strong>
+              <CopyIcon
+                className="DiffNode--copyIcon"
+                copyText={`${service} ${operation}`}
+                tooltipTitle="Copy label"
+              />
             </td>
           </tr>
           <tr>

packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css

@@ -46,6 +46,16 @@ limitations under the License.
   padding: 0.3rem 0.5rem 0.3rem 0.75rem;
 }
 
+.OpNode--popover .OpNode--copyIcon,
+.OpNode:not(:hover) .OpNode--copyIcon {
+  display: none;
+}
+
+.OpNode--service {
+  display: flex;
+  justify-content: space-between;
+}
+
 /* Tweak the popover aesthetics - unfortunate but necessary */
 
 .OpNode--popover .ant-popover-inner-content {

packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js

@@ -16,6 +16,8 @@
 
 import * as React from 'react';
 import { Popover } from 'antd';
+
+import CopyIcon from '../../common/CopyIcon';
 import colorGenerator from '../../../utils/color-generator';
 
 import type { PVertex } from '../../../model/trace-dag/types';
@@ -94,6 +96,11 @@ export default class OpNode extends React.PureComponent<Props> {
             </td>
             <td className="OpNode--labelCell OpNode--service">
               <strong>{service}</strong>
+              <CopyIcon
+                className="OpNode--copyIcon"
+                copyText={`${service} ${operation}`}
+                tooltipTitle="Copy label"
+              />
             </td>
             <td className="OpNode--metricCell OpNode--avg">{round2(time / 1000 / count)} ms</td>
           </tr>

packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js

@@ -16,6 +16,7 @@ import React from 'react';
 import { shallow } from 'enzyme';
 
 import OpNode, { getNodeDrawer, MODE_SERVICE, MODE_TIME, MODE_SELFTIME } from './OpNode';
+import CopyIcon from '../../common/CopyIcon';
 
 describe('<OpNode>', () => {
   let wrapper;
@@ -49,7 +50,12 @@ describe('<OpNode>', () => {
     expect(wrapper.find('.OpNode--avg').text()).toBe('40 ms');
     expect(wrapper.find('.OpNode--selfTime').text()).toBe('180 ms (90 %)');
     expect(wrapper.find('.OpNode--op').text()).toBe('op1');
-    expect(wrapper.find('.OpNode--service').text()).toBe('service1');
+    expect(
+      wrapper
+        .find('.OpNode--service')
+        .find('strong')
+        .text()
+    ).toBe('service1');
   });
 
   it('it switches mode', () => {
@@ -72,6 +78,13 @@ describe('<OpNode>', () => {
     expect(wrapper.find('.OpNode--mode-selftime').length).toBe(1);
   });
 
+  it('renders a copy icon', () => {
+    const copyIcon = wrapper.find(CopyIcon);
+    expect(copyIcon.length).toBe(1);
+    expect(copyIcon.prop('copyText')).toBe(`${props.service} ${props.operation}`);
+    expect(copyIcon.prop('tooltipTitle')).toBe('Copy label');
+  });
+
   describe('getNodeDrawer()', () => {
     it('it creates OpNode', () => {
       const vertex = {

packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css

@@ -44,12 +44,8 @@ limitations under the License.
   text-align: right;
 }
 
-.KeyValueTable--copyIcon {
-  visibility: hidden;
-}
-
-.KeyValueTable--row:hover > .KeyValueTable--copyColumn > .KeyValueTable--copyIcon {
-  visibility: unset;
+.KeyValueTable--row:not(:hover) > .KeyValueTable--copyColumn > .KeyValueTable--copyIcon {
+  display: none;
 }
 
 .KeyValueTable--row > td {

packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js

@@ -16,18 +16,17 @@
 
 import * as React from 'react';
 import jsonMarkup from 'json-markup';
-import { Dropdown, Icon, Menu, Tooltip } from 'antd';
-import { CopyToClipboard } from 'react-copy-to-clipboard';
+import { Dropdown, Icon, Menu } from 'antd';
+
+import CopyIcon from '../../../common/CopyIcon';
+
 import type { KeyValuePair, Link } from '../../../../types/trace';
 
 import './KeyValuesTable.css';
 
 function parseIfJson(value) {
   try {
-    const data = JSON.parse(value);
-    if (data && typeof data === 'object') {
-      return data;
-    }
+    return JSON.parse(value);
     // eslint-disable-next-line no-empty
   } catch (_) {}
   return value;
@@ -60,104 +59,59 @@ type KeyValuesTableProps = {
   linksGetter: ?(KeyValuePair[], number) => Link[],
 };
 
-type KeyValuesTableState = {
-  copiedRows: Set<KeyValuePair>,
-};
-
-export default class KeyValuesTable extends React.PureComponent<KeyValuesTableProps, KeyValuesTableState> {
-  props: KeyValuesTableProps;
-
-  constructor(props: KeyValuesTableProps) {
-    super(props);
-
-    this.state = {
-      copiedRows: new Set(),
-    };
-  }
-
-  handleCopyIconClick = (row: KeyValuePair) => {
-    const newCopiedRows = new Set(this.state.copiedRows);
-    newCopiedRows.add(row);
-    this.setState({
-      copiedRows: newCopiedRows,
-    });
-  };
-
-  handleTooltipVisibilityChange = (row: KeyValuePair, visible: boolean) => {
-    if (!visible && this.state.copiedRows.has(row)) {
-      const newCopiedRows = new Set(this.state.copiedRows);
-      newCopiedRows.delete(row);
-      this.setState({
-        copiedRows: newCopiedRows,
-      });
-    }
-  };
-
-  render() {
-    const { data, linksGetter } = this.props;
-    return (
-      <div className="KeyValueTable u-simple-scrollbars">
-        <table className="u-width-100">
-          <tbody className="KeyValueTable--body">
-            {data.map((row, i) => {
-              const tooltipTitle = this.state.copiedRows.has(row) ? 'Copied' : 'Copy JSON';
-              const markup = {
-                __html: jsonMarkup(parseIfJson(row.value)),
-              };
-              // eslint-disable-next-line react/no-danger
-              const jsonTable = <div className="ub-inline-block" dangerouslySetInnerHTML={markup} />;
-              const links = linksGetter ? linksGetter(data, i) : null;
-              let valueMarkup;
-              if (links && links.length === 1) {
-                valueMarkup = (
-                  <div>
-                    <LinkValue href={links[0].url} title={links[0].text}>
-                      {jsonTable}
-                    </LinkValue>
-                  </div>
-                );
-              } else if (links && links.length > 1) {
-                valueMarkup = (
-                  <div>
-                    <Dropdown overlay={linkValueList(links)} placement="bottomRight" trigger={['click']}>
-                      <a>
-                        {jsonTable} <Icon className="KeyValueTable--linkIcon" type="profile" />
-                      </a>
-                    </Dropdown>
-                  </div>
-                );
-              } else {
-                valueMarkup = jsonTable;
-              }
-              return (
-                // `i` is necessary in the key because row.key can repeat
-                // eslint-disable-next-line react/no-array-index-key
-                <tr className="KeyValueTable--row" key={`${row.key}-${i}`}>
-                  <td className="KeyValueTable--keyColumn">{row.key}</td>
-                  <td>{valueMarkup}</td>
-                  <td className="KeyValueTable--copyColumn">
-                    <Tooltip
-                      arrowPointAtCenter
-                      mouseLeaveDelay={0.5}
-                      onVisibleChange={visible => this.handleTooltipVisibilityChange(row, visible)}
-                      placement="left"
-                      title={tooltipTitle}
-                    >
-                      <CopyToClipboard text={JSON.stringify(row, null, 2)}>
-                        <Icon
-                          className="KeyValueTable--copyIcon"
-                          onClick={() => this.handleCopyIconClick(row)}
-                          type="copy"
-                        />
-                      </CopyToClipboard>
-                    </Tooltip>
-                  </td>
-                </tr>
+export default function KeyValuesTable(props: KeyValuesTableProps) {
+  const { data, linksGetter } = props;
+  return (
+    <div className="KeyValueTable u-simple-scrollbars">
+      <table className="u-width-100">
+        <tbody className="KeyValueTable--body">
+          {data.map((row, i) => {
+            const markup = {
+              __html: jsonMarkup(parseIfJson(row.value)),
+            };
+            // eslint-disable-next-line react/no-danger
+            const jsonTable = <div className="ub-inline-block" dangerouslySetInnerHTML={markup} />;
+            const links = linksGetter ? linksGetter(data, i) : null;
+            let valueMarkup;
+            if (links && links.length === 1) {
+              valueMarkup = (
+                <div>
+                  <LinkValue href={links[0].url} title={links[0].text}>
+                    {jsonTable}
+                  </LinkValue>
+                </div>
+              );
+            } else if (links && links.length > 1) {
+              valueMarkup = (
+                <div>
+                  <Dropdown overlay={linkValueList(links)} placement="bottomRight" trigger={['click']}>
+                    <a>
+                      {jsonTable} <Icon className="KeyValueTable--linkIcon" type="profile" />
+                    </a>
+                  </Dropdown>
+                </div>
               );
-            })}
-          </tbody>
-        </table>
-      </div>
-    );
-  }
+            } else {
+              valueMarkup = jsonTable;
+            }
+            return (
+              // `i` is necessary in the key because row.key can repeat
+              // eslint-disable-next-line react/no-array-index-key
+              <tr className="KeyValueTable--row" key={`${row.key}-${i}`}>
+                <td className="KeyValueTable--keyColumn">{row.key}</td>
+                <td>{valueMarkup}</td>
+                <td className="KeyValueTable--copyColumn">
+                  <CopyIcon
+                    className="KeyValueTable--copyIcon"
+                    copyText={JSON.stringify(row, null, 2)}
+                    tooltipTitle="Copy JSON"
+                  />
+                </td>
+              </tr>
+            );
+          })}
+        </tbody>
+      </table>
+    </div>
+  );
 }

packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js

@@ -14,11 +14,34 @@
 
 import React from 'react';
 import { shallow } from 'enzyme';
-import { Dropdown, Icon, Tooltip } from 'antd';
-import { CopyToClipboard } from 'react-copy-to-clipboard';
+import { Dropdown, Icon } from 'antd';
+
+import CopyIcon from '../../../common/CopyIcon';
 
 import KeyValuesTable, { LinkValue } from './KeyValuesTable';
 
+describe('LinkValue', () => {
+  const title = 'titleValue';
+  const href = 'hrefValue';
+  const childrenText = 'childrenTextValue';
+  const wrapper = shallow(
+    <LinkValue href={href} title={title}>
+      {childrenText}
+    </LinkValue>
+  );
+
+  it('renders as expected', () => {
+    expect(wrapper.find('a').prop('href')).toBe(href);
+    expect(wrapper.find('a').prop('title')).toBe(title);
+    expect(wrapper.find('a').text()).toMatch(/childrenText/);
+  });
+
+  it('renders correct Icon', () => {
+    expect(wrapper.find(Icon).hasClass('KeyValueTable--linkIcon')).toBe(true);
+    expect(wrapper.find(Icon).prop('type')).toBe('export');
+  });
+});
+
 describe('<KeyValuesTable>', () => {
   let wrapper;
 
@@ -96,49 +119,12 @@ describe('<KeyValuesTable>', () => {
     ).toBe('span.kind');
   });
 
-  describe('CopyIcon', () => {
-    const indexToCopy = 1;
-
-    it('should render a Copy icon with <CopyToClipboard /> and <Tooltip /> for each data element', () => {
-      const trs = wrapper.find('tr');
-      expect(trs.length).toBe(data.length);
-      trs.forEach((tr, i) => {
-        const copyColumn = tr.find('.KeyValueTable--copyColumn');
-        expect(copyColumn.find(CopyToClipboard).prop('text')).toBe(JSON.stringify(data[i], null, 2));
-        expect(copyColumn.find(Tooltip).length).toBe(1);
-        expect(copyColumn.find({ type: 'copy' }).length).toBe(1);
-      });
-    });
-
-    it('should add correct data entry to state when icon is clicked', () => {
-      expect(wrapper.state().copiedRows.size).toBe(0);
-      wrapper
-        .find('tr')
-        .at(indexToCopy)
-        .find(Icon)
-        .simulate('click');
-      expect(wrapper.state().copiedRows.size).toBe(1);
-      expect(wrapper.state().copiedRows.has(data[indexToCopy])).toBe(true);
-    });
-
-    it('should remove correct data entry to state when tooltip hides', () => {
-      wrapper.setState({ copiedRows: new Set(data) });
-      wrapper
-        .find('tr')
-        .at(indexToCopy)
-        .find(Tooltip)
-        .prop('onVisibleChange')(false);
-      expect(wrapper.state().copiedRows.size).toBe(data.length - 1);
-      expect(wrapper.state().copiedRows.has(data[indexToCopy])).toBe(false);
-    });
-
-    it('should render correct tooltip title for each row', () => {
-      wrapper.setState({ copiedRows: new Set([data[indexToCopy]]) });
-      const tooltips = wrapper.find(Tooltip);
-      tooltips.forEach((tooltip, i) =>
-        expect(tooltip.prop('title')).toBe(i === indexToCopy ? 'Copied' : 'Copy JSON')
-      );
-      expect.assertions(data.length);
+  it('renders a <CopyIcon /> with correct copyText for each data element', () => {
+    const copyIcons = wrapper.find(CopyIcon);
+    expect(copyIcons.length).toBe(data.length);
+    copyIcons.forEach((copyIcon, i) => {
+      expect(copyIcon.prop('copyText')).toBe(JSON.stringify(data[i], null, 2));
+      expect(copyIcon.prop('tooltipTitle')).toBe('Copy JSON');
     });
   });
 });