From 2949ebc2647fc20aa596749987adde4587957212 Mon Sep 17 00:00:00 2001 From: svennergr Date: Fri, 20 May 2022 20:24:33 +0200 Subject: [PATCH] Traces: Fixed missing CopyButton on KeyValueTables and overlapping of panels (#49271) * rewrote `CopyIcon` to an functional component - fixed missing `CopyIcon` on `KeyValuesTable` * added fixed height of `30px` to `KeyValuesTable` to fix overlapping * removed unused CopyIcon snapshot * KeyValuesTable: moved `30px` height from `tr` to `td` to fix vertical alignment --- .betterer.results | 3 - .../SpanDetail/KeyValuesTable.tsx | 1 + .../src/common/CopyIcon.test.js | 23 ++----- .../src/common/CopyIcon.tsx | 65 ++++++------------- .../__snapshots__/CopyIcon.test.js.snap | 14 ---- 5 files changed, 29 insertions(+), 77 deletions(-) delete mode 100644 packages/jaeger-ui-components/src/common/__snapshots__/CopyIcon.test.js.snap diff --git a/.betterer.results b/.betterer.results index 6e5f4a6438b..e8cf5c8ca7e 100644 --- a/.betterer.results +++ b/.betterer.results @@ -110,9 +110,6 @@ exports[`no enzyme tests`] = { "packages/jaeger-ui-components/src/TraceTimelineViewer/index.test.js:381298544": [ [14, 19, 13, "RegExp match", "2409514259"] ], - "packages/jaeger-ui-components/src/common/CopyIcon.test.js:187212136": [ - [15, 19, 13, "RegExp match", "2409514259"] - ], "packages/jaeger-ui-components/src/common/NewWindowIcon.test.js:1750458349": [ [14, 19, 13, "RegExp match", "2409514259"] ], diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx index 9e17ff31070..5c3df6b6e69 100644 --- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx +++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx @@ -46,6 +46,7 @@ export const getStyles = (theme: GrafanaTheme2) => { label: row; & > td { padding: 0rem 0.5rem; + height: 30px; } &:nth-child(2n) > td { background: ${autoColor(theme, '#f5f5f5')}; diff --git a/packages/jaeger-ui-components/src/common/CopyIcon.test.js b/packages/jaeger-ui-components/src/common/CopyIcon.test.js index 10ea26789c2..b06ac040043 100644 --- a/packages/jaeger-ui-components/src/common/CopyIcon.test.js +++ b/packages/jaeger-ui-components/src/common/CopyIcon.test.js @@ -12,12 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +import { render, screen } from '@testing-library/react'; import * as copy from 'copy-to-clipboard'; -import { shallow } from 'enzyme'; import React from 'react'; -import { Button, Tooltip } from '@grafana/ui'; - import CopyIcon from './CopyIcon'; jest.mock('copy-to-clipboard'); @@ -29,7 +27,6 @@ describe('', () => { tooltipTitle: 'tooltipTitleValue', }; let copySpy; - let wrapper; beforeAll(() => { copySpy = jest.spyOn(copy, 'default'); @@ -37,24 +34,18 @@ describe('', () => { beforeEach(() => { copySpy.mockReset(); - wrapper = shallow(); }); it('renders as expected', () => { - expect(wrapper).toMatchSnapshot(); + expect(() => render()).not.toThrow(); }); - it('updates state and copies when clicked', () => { - expect(wrapper.state().hasCopied).toBe(false); - expect(copySpy).not.toHaveBeenCalled(); + it('copies when clicked', () => { + render(); + + const button = screen.getByRole('button'); + button.click(); - wrapper.find(Button).simulate('click'); - expect(wrapper.state().hasCopied).toBe(true); expect(copySpy).toHaveBeenCalledWith(props.copyText); }); - - it('persists state when tooltip opens', () => { - wrapper.setState({ hasCopied: true }); - expect(wrapper.state().hasCopied).toBe(true); - }); }); diff --git a/packages/jaeger-ui-components/src/common/CopyIcon.tsx b/packages/jaeger-ui-components/src/common/CopyIcon.tsx index 6c674713045..5fb166a7ada 100644 --- a/packages/jaeger-ui-components/src/common/CopyIcon.tsx +++ b/packages/jaeger-ui-components/src/common/CopyIcon.tsx @@ -15,11 +15,11 @@ import { css } from '@emotion/css'; import cx from 'classnames'; import copy from 'copy-to-clipboard'; -import * as React from 'react'; +import React, { useState } from 'react'; -import { Button, IconName, stylesFactory, Tooltip } from '@grafana/ui'; +import { Button, IconName, Tooltip, useStyles2 } from '@grafana/ui'; -const getStyles = stylesFactory(() => { +const getStyles = () => { return { CopyIcon: css` background-color: transparent; @@ -27,14 +27,13 @@ const getStyles = stylesFactory(() => { color: inherit; height: 100%; overflow: hidden; - padding: 0px; &:focus { background-color: rgba(255, 255, 255, 0.25); color: inherit; } `, }; -}); +}; type PropsType = { className?: string; @@ -43,46 +42,24 @@ type PropsType = { tooltipTitle: string; }; -type StateType = { - hasCopied: boolean; -}; +export default function CopyIcon(props: PropsType) { + const styles = useStyles2(getStyles); -export default class CopyIcon extends React.PureComponent { - static defaultProps: Partial = { - className: undefined, - icon: 'copy', + const [hasCopied, setHasCopied] = useState(false); + + const handleClick = () => { + copy(props.copyText); + setHasCopied(true); }; - state = { - hasCopied: false, - }; - - handleClick = () => { - this.setState({ - hasCopied: true, - }); - copy(this.props.copyText); - }; - - handleTooltipVisibilityChange = (visible: boolean) => { - if (!visible && this.state.hasCopied) { - this.setState({ - hasCopied: false, - }); - } - }; - - render() { - const styles = getStyles(); - return ( - -