From e7025b4da4a1f0c5ce14b9617c2712268fb4dc64 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Thu, 10 Jan 2019 17:59:13 -0500 Subject: [PATCH 1/6] * Refactor KV Table copy icon into standalone component Signed-off-by: Everett Ross --- .../SpanDetail/KeyValuesTable.css | 8 +- .../SpanDetail/KeyValuesTable.js | 158 +++++++----------- .../SpanDetail/KeyValuesTable.test.js | 54 +----- .../__snapshots__/copy-icon.test.js.snap | 24 +++ .../src/components/common/copy-icon.js | 70 ++++++++ .../src/components/common/copy-icon.test.js | 59 +++++++ 6 files changed, 221 insertions(+), 152 deletions(-) create mode 100644 packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap create mode 100644 packages/jaeger-ui/src/components/common/copy-icon.js create mode 100644 packages/jaeger-ui/src/components/common/copy-icon.test.js diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css index 04a80ad144..07be25c6b0 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css +++ b/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 { diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 34344cd4f9..207fa43745 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -16,8 +16,10 @@ 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/copy-icon'; + import type { KeyValuePair, Link } from '../../../../types/trace'; import './KeyValuesTable.css'; @@ -60,104 +62,58 @@ type KeyValuesTableProps = { linksGetter: ?(KeyValuePair[], number) => Link[], }; -type KeyValuesTableState = { - copiedRows: Set, -}; - -export default class KeyValuesTable extends React.PureComponent { - 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 ( -
- - - {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 =
; - const links = linksGetter ? linksGetter(data, i) : null; - let valueMarkup; - if (links && links.length === 1) { - valueMarkup = ( -
- - {jsonTable} - -
- ); - } else if (links && links.length > 1) { - valueMarkup = ( - - ); - } else { - valueMarkup = jsonTable; - } - return ( - // `i` is necessary in the key because row.key can repeat - // eslint-disable-next-line react/no-array-index-key -
- - - - +export default function KeyValuesTable(props: KeyValuesTableProps) { + const { data, linksGetter } = props; + return ( +
+
{row.key}{valueMarkup} - this.handleTooltipVisibilityChange(row, visible)} - placement="left" - title={tooltipTitle} - > - - this.handleCopyIconClick(row)} - type="copy" - /> - - -
+ + {data.map((row, i) => { + const markup = { + __html: jsonMarkup(parseIfJson(row.value)), + }; + // eslint-disable-next-line react/no-danger + const jsonTable =
; + const links = linksGetter ? linksGetter(data, i) : null; + let valueMarkup; + if (links && links.length === 1) { + valueMarkup = ( +
+ + {jsonTable} + +
+ ); + } else if (links && links.length > 1) { + valueMarkup = ( + ); - })} -
-
-
- ); - } + } else { + valueMarkup = jsonTable; + } + return ( + // `i` is necessary in the key because row.key can repeat + // eslint-disable-next-line react/no-array-index-key + + {row.key} + {valueMarkup} + +
+ +
+ + + ); + })} + + + + ); } +// } diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index 5ece890e86..288401de21 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -14,8 +14,9 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Dropdown, Icon, Tooltip } from 'antd'; -import { CopyToClipboard } from 'react-copy-to-clipboard'; +import { Dropdown } from 'antd'; + +import CopyIcon from '../../../common/copy-icon'; import KeyValuesTable, { LinkValue } from './KeyValuesTable'; @@ -96,49 +97,12 @@ describe('', () => { ).toBe('span.kind'); }); - describe('CopyIcon', () => { - const indexToCopy = 1; - - it('should render a Copy icon with and 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 with correct copyText for each data element', () => { + const copyIcons = wrapper.find(CopyIcon); + expect(copyIcons.length).toBe(data.length); + copyIcons.forEach((copyColumn, i) => { + expect(copyColumn.prop('copyText')).toBe(JSON.stringify(data[i], null, 2)); + expect(copyColumn.prop('tooltipTitle')).toBe('Copy JSON'); }); }); }); diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap new file mode 100644 index 0000000000..a08b7182b8 --- /dev/null +++ b/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap @@ -0,0 +1,24 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders as expected 1`] = ` + + + + + +`; diff --git a/packages/jaeger-ui/src/components/common/copy-icon.js b/packages/jaeger-ui/src/components/common/copy-icon.js new file mode 100644 index 0000000000..d2a4e92c71 --- /dev/null +++ b/packages/jaeger-ui/src/components/common/copy-icon.js @@ -0,0 +1,70 @@ +// @flow + +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as React from 'react'; + +import { Icon, Tooltip } from 'antd'; +import { CopyToClipboard } from 'react-copy-to-clipboard'; + +type propsType = { + copyText: string, + tooltipTitle: string, +}; + +type stateType = { + hasCopied: boolean, +}; + +export default class CopyIcon extends React.PureComponent { + constructor(props: propsType) { + super(props); + + this.state = { + hasCopied: false, + }; + } + + handleCopyIconClick = () => { + this.setState({ + hasCopied: true, + }); + }; + + handleTooltipVisibilityChange = (visible: boolean) => { + if (!visible) { + this.setState({ + hasCopied: false, + }); + } + }; + + render() { + return ( + + + + ); + } +} diff --git a/packages/jaeger-ui/src/components/common/copy-icon.test.js b/packages/jaeger-ui/src/components/common/copy-icon.test.js new file mode 100644 index 0000000000..950e6794b2 --- /dev/null +++ b/packages/jaeger-ui/src/components/common/copy-icon.test.js @@ -0,0 +1,59 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import React from 'react'; +import { shallow } from 'enzyme'; +import { Icon, Tooltip } from 'antd'; + +import CopyIcon from './copy-icon'; + +describe('', () => { + const props = { + copyText: 'copyTextValue', + tooltipTitle: 'tooltipTitleValue', + }; + let wrapper; + + beforeEach(() => { + wrapper = shallow(); + }); + + it('renders as expected', () => { + expect(wrapper).toMatchSnapshot(); + }); + + it('updates state when icon is clicked', () => { + expect(wrapper.state().hasCopied).toBe(false); + wrapper + .find(Icon) + .simulate('click'); + expect(wrapper.state().hasCopied).toBe(true); + }); + + it('updates state when tooltip hides', () => { + wrapper.setState({ hasCopied: true }); + wrapper + .find(Tooltip) + .prop('onVisibleChange')(false); + expect(wrapper.state().hasCopied).toBe(false); + }); + + it('persists state when tooltip opens', () => { + wrapper.setState({ hasCopied: true }); + wrapper + .find(Tooltip) + .prop('onVisibleChange')(true); + expect(wrapper.state().hasCopied).toBe(true); + }); +}); From 0afd7ac81fcd2c1aee279959dd4295aa829a6410 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Thu, 10 Jan 2019 18:01:34 -0500 Subject: [PATCH 2/6] Adhere to prettier rules Signed-off-by: Everett Ross --- .../src/components/common/copy-icon.js | 27 +++++++++---------- .../src/components/common/copy-icon.test.js | 12 +++------ 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/copy-icon.js b/packages/jaeger-ui/src/components/common/copy-icon.js index d2a4e92c71..e14a8c7ee4 100644 --- a/packages/jaeger-ui/src/components/common/copy-icon.js +++ b/packages/jaeger-ui/src/components/common/copy-icon.js @@ -52,19 +52,18 @@ export default class CopyIcon extends React.PureComponent }; render() { - return ( - - - - ); + return ( + + + + + + ); } } diff --git a/packages/jaeger-ui/src/components/common/copy-icon.test.js b/packages/jaeger-ui/src/components/common/copy-icon.test.js index 950e6794b2..02186a9aa8 100644 --- a/packages/jaeger-ui/src/components/common/copy-icon.test.js +++ b/packages/jaeger-ui/src/components/common/copy-icon.test.js @@ -35,25 +35,19 @@ describe('', () => { it('updates state when icon is clicked', () => { expect(wrapper.state().hasCopied).toBe(false); - wrapper - .find(Icon) - .simulate('click'); + wrapper.find(Icon).simulate('click'); expect(wrapper.state().hasCopied).toBe(true); }); it('updates state when tooltip hides', () => { wrapper.setState({ hasCopied: true }); - wrapper - .find(Tooltip) - .prop('onVisibleChange')(false); + wrapper.find(Tooltip).prop('onVisibleChange')(false); expect(wrapper.state().hasCopied).toBe(false); }); it('persists state when tooltip opens', () => { wrapper.setState({ hasCopied: true }); - wrapper - .find(Tooltip) - .prop('onVisibleChange')(true); + wrapper.find(Tooltip).prop('onVisibleChange')(true); expect(wrapper.state().hasCopied).toBe(true); }); }); From 2301b493455340fb633671f06ee867ac92b7e84c Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Fri, 11 Jan 2019 10:55:22 -0500 Subject: [PATCH 3/6] Add to drawNode and OpNode Signed-off-by: Everett Ross --- .../TraceDiff/TraceDiffGraph/drawNode.css | 10 ++++++++++ .../TraceDiff/TraceDiffGraph/drawNode.js | 5 +++++ .../components/TracePage/TraceGraph/OpNode.css | 10 ++++++++++ .../src/components/TracePage/TraceGraph/OpNode.js | 5 +++++ .../TracePage/TraceGraph/OpNode.test.js | 15 ++++++++++++++- 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css index 13c1c79fd9..3b47e0940e 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css @@ -74,6 +74,16 @@ limitations under the License. padding: 0.3rem 0.5rem 0.3rem 0.75rem; } +.DiffNode--popover .DiffNode--copyIcon, +.DiffNode:not(:hover) .DiffNode--copyIcon { + display: none; +} + +.DiffNode--labelCell { + display: flex; + justify-content: space-between; +} + /* Tweak the popover aesthetics - unfortunate but necessary */ .DiffNode--popover .ant-popover-inner-content { diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js index cbe2d3b9f6..5f234e02a0 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js +++ b/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/copy-icon'; + import type { PVertex } from '../../../model/trace-dag/types'; import './drawNode.css'; @@ -57,6 +59,9 @@ class DiffNode extends React.PureComponent { {service} + + + diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css index 75e2a545cf..f34560e4dd 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css +++ b/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 { diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js index 43d7df6bf4..9b63856cc4 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js +++ b/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/copy-icon'; import colorGenerator from '../../../utils/color-generator'; import type { PVertex } from '../../../model/trace-dag/types'; @@ -94,6 +96,9 @@ export default class OpNode extends React.PureComponent { {service} + + + {round2(time / 1000 / count)} ms diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js index 4d10931edd..c3e5e17223 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js +++ b/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/copy-icon'; describe('', () => { let wrapper; @@ -49,7 +50,12 @@ describe('', () => { 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('', () => { 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 = { From 046de99a5d384cf62f29eb27db1f0c49351c55c9 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Fri, 11 Jan 2019 10:59:59 -0500 Subject: [PATCH 4/6] Improve variable names for splitting CopyIcon into own file Signed-off-by: Everett Ross --- .../TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js | 6 +++--- packages/jaeger-ui/src/components/common/copy-icon.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index 288401de21..439cac1376 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -100,9 +100,9 @@ describe('', () => { it('renders a with correct copyText for each data element', () => { const copyIcons = wrapper.find(CopyIcon); expect(copyIcons.length).toBe(data.length); - copyIcons.forEach((copyColumn, i) => { - expect(copyColumn.prop('copyText')).toBe(JSON.stringify(data[i], null, 2)); - expect(copyColumn.prop('tooltipTitle')).toBe('Copy JSON'); + copyIcons.forEach((copyIcon, i) => { + expect(copyIcon.prop('copyText')).toBe(JSON.stringify(data[i], null, 2)); + expect(copyIcon.prop('tooltipTitle')).toBe('Copy JSON'); }); }); }); diff --git a/packages/jaeger-ui/src/components/common/copy-icon.js b/packages/jaeger-ui/src/components/common/copy-icon.js index e14a8c7ee4..c2f88e3c7d 100644 --- a/packages/jaeger-ui/src/components/common/copy-icon.js +++ b/packages/jaeger-ui/src/components/common/copy-icon.js @@ -37,7 +37,7 @@ export default class CopyIcon extends React.PureComponent }; } - handleCopyIconClick = () => { + handleClick = () => { this.setState({ hasCopied: true, }); @@ -61,7 +61,7 @@ export default class CopyIcon extends React.PureComponent title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle} > - + ); From c9dec8642354e5ec55fde1758fefdb8a5c2776d5 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Wed, 16 Jan 2019 19:00:25 -0500 Subject: [PATCH 5/6] Clean up CopyIcon Signed-off-by: Everett Ross --- .../TraceDiff/TraceDiffGraph/drawNode.css | 7 ++---- .../TraceDiff/TraceDiffGraph/drawNode.js | 10 +++++--- .../components/TracePage/TraceGraph/OpNode.js | 10 +++++--- .../TracePage/TraceGraph/OpNode.test.js | 2 +- .../SpanDetail/KeyValuesTable.js | 10 +++++--- .../SpanDetail/KeyValuesTable.test.js | 2 +- .../common/{copy-icon.js => CopyIcon.js} | 24 +++++++++--------- .../{copy-icon.test.js => CopyIcon.test.js} | 9 +++++-- .../__snapshots__/CopyIcon.test.js.snap | 25 +++++++++++++++++++ 9 files changed, 66 insertions(+), 33 deletions(-) rename packages/jaeger-ui/src/components/common/{copy-icon.js => CopyIcon.js} (79%) rename packages/jaeger-ui/src/components/common/{copy-icon.test.js => CopyIcon.test.js} (84%) create mode 100644 packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css index 3b47e0940e..e89c92416f 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.css @@ -72,6 +72,8 @@ 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, @@ -79,11 +81,6 @@ limitations under the License. display: none; } -.DiffNode--labelCell { - display: flex; - justify-content: space-between; -} - /* Tweak the popover aesthetics - unfortunate but necessary */ .DiffNode--popover .ant-popover-inner-content { diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js index 5f234e02a0..1b7bc97b4b 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/drawNode.js @@ -18,7 +18,7 @@ import * as React from 'react'; import { Popover } from 'antd'; import cx from 'classnames'; -import CopyIcon from '../../common/copy-icon'; +import CopyIcon from '../../common/CopyIcon'; import type { PVertex } from '../../../model/trace-dag/types'; @@ -59,9 +59,11 @@ class DiffNode extends React.PureComponent { {service} - - - + diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js index 9b63856cc4..7d757964dc 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js @@ -17,7 +17,7 @@ import * as React from 'react'; import { Popover } from 'antd'; -import CopyIcon from '../../common/copy-icon'; +import CopyIcon from '../../common/CopyIcon'; import colorGenerator from '../../../utils/color-generator'; import type { PVertex } from '../../../model/trace-dag/types'; @@ -96,9 +96,11 @@ export default class OpNode extends React.PureComponent { {service} - - - + {round2(time / 1000 / count)} ms diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js index c3e5e17223..701e92959b 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js @@ -16,7 +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/copy-icon'; +import CopyIcon from '../../common/CopyIcon'; describe('', () => { let wrapper; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 207fa43745..056858ab70 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -18,7 +18,7 @@ import * as React from 'react'; import jsonMarkup from 'json-markup'; import { Dropdown, Icon, Menu } from 'antd'; -import CopyIcon from '../../../common/copy-icon'; +import CopyIcon from '../../../common/CopyIcon'; import type { KeyValuePair, Link } from '../../../../types/trace'; @@ -104,9 +104,11 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { {row.key} {valueMarkup} -
- -
+ ); diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index 439cac1376..4183d76ade 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -16,7 +16,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { Dropdown } from 'antd'; -import CopyIcon from '../../../common/copy-icon'; +import CopyIcon from '../../../common/CopyIcon'; import KeyValuesTable, { LinkValue } from './KeyValuesTable'; diff --git a/packages/jaeger-ui/src/components/common/copy-icon.js b/packages/jaeger-ui/src/components/common/CopyIcon.js similarity index 79% rename from packages/jaeger-ui/src/components/common/copy-icon.js rename to packages/jaeger-ui/src/components/common/CopyIcon.js index c2f88e3c7d..2030e85538 100644 --- a/packages/jaeger-ui/src/components/common/copy-icon.js +++ b/packages/jaeger-ui/src/components/common/CopyIcon.js @@ -19,23 +19,23 @@ import * as React from 'react'; import { Icon, Tooltip } from 'antd'; import { CopyToClipboard } from 'react-copy-to-clipboard'; -type propsType = { +type PropsType = { + className?: string, copyText: string, tooltipTitle: string, }; -type stateType = { +type StateType = { hasCopied: boolean, }; -export default class CopyIcon extends React.PureComponent { - constructor(props: propsType) { - super(props); - - this.state = { - hasCopied: false, - }; - } +export default class CopyIcon extends React.PureComponent { + static defaultProps = { + className: null, + }; + state = { + hasCopied: false, + }; handleClick = () => { this.setState({ @@ -44,7 +44,7 @@ export default class CopyIcon extends React.PureComponent }; handleTooltipVisibilityChange = (visible: boolean) => { - if (!visible) { + if (!visible && this.state.hasCopied) { this.setState({ hasCopied: false, }); @@ -61,7 +61,7 @@ export default class CopyIcon extends React.PureComponent title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle} > - + ); diff --git a/packages/jaeger-ui/src/components/common/copy-icon.test.js b/packages/jaeger-ui/src/components/common/CopyIcon.test.js similarity index 84% rename from packages/jaeger-ui/src/components/common/copy-icon.test.js rename to packages/jaeger-ui/src/components/common/CopyIcon.test.js index 02186a9aa8..f94adacfac 100644 --- a/packages/jaeger-ui/src/components/common/copy-icon.test.js +++ b/packages/jaeger-ui/src/components/common/CopyIcon.test.js @@ -16,10 +16,11 @@ import React from 'react'; import { shallow } from 'enzyme'; import { Icon, Tooltip } from 'antd'; -import CopyIcon from './copy-icon'; +import CopyIcon from './CopyIcon'; describe('', () => { const props = { + className: 'classNameValue', copyText: 'copyTextValue', tooltipTitle: 'tooltipTitleValue', }; @@ -39,10 +40,14 @@ describe('', () => { expect(wrapper.state().hasCopied).toBe(true); }); - it('updates state when tooltip hides', () => { + it('updates state when tooltip hides and state.hasCopied is true', () => { wrapper.setState({ hasCopied: true }); wrapper.find(Tooltip).prop('onVisibleChange')(false); expect(wrapper.state().hasCopied).toBe(false); + + const state = wrapper.state(); + wrapper.find(Tooltip).prop('onVisibleChange')(false); + expect(wrapper.state()).toBe(state); }); it('persists state when tooltip opens', () => { diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap new file mode 100644 index 0000000000..4f077742e3 --- /dev/null +++ b/packages/jaeger-ui/src/components/common/__snapshots__/CopyIcon.test.js.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders as expected 1`] = ` + + + + + +`; From 5269cc38d0a84da548423f76a0338847a2100469 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Thu, 17 Jan 2019 11:46:00 -0500 Subject: [PATCH 6/6] Fix KeyValuesTable test coverage Signed-off-by: Everett Ross --- .../SpanDetail/KeyValuesTable.js | 6 +---- .../SpanDetail/KeyValuesTable.test.js | 24 ++++++++++++++++++- .../__snapshots__/copy-icon.test.js.snap | 24 ------------------- 3 files changed, 24 insertions(+), 30 deletions(-) delete mode 100644 packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 056858ab70..7fda65d5aa 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -26,10 +26,7 @@ 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; @@ -118,4 +115,3 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { ); } -// } diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index 4183d76ade..c1326f9f24 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -14,12 +14,34 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Dropdown } from 'antd'; +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( + + {childrenText} + + ); + + 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('', () => { let wrapper; diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap deleted file mode 100644 index a08b7182b8..0000000000 --- a/packages/jaeger-ui/src/components/common/__snapshots__/copy-icon.test.js.snap +++ /dev/null @@ -1,24 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[` renders as expected 1`] = ` - - - - - -`;