Skip to content

Commit e2804bb

Browse files
sroy3mattseddon
andauthored
Use copy button component for the plots ribbon experiments (#1812)
* Add plots ribbon * Fix tests and event name * Add tests * Add react-virtualized * Add extension test * Add button in plots ribbon to add experiments * Add tests * Add refresh all button * add refresh all visible action to plots (#1808) * Use copy button instead of creating new copy function * Fix tests * Fix format after merge Co-authored-by: mattseddon <[email protected]>
1 parent 1779f06 commit e2804bb

File tree

8 files changed

+67
-133
lines changed

8 files changed

+67
-133
lines changed

webview/src/experiments/components/table/styles.module.scss

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -478,18 +478,7 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding;
478478
}
479479

480480
.copyButton {
481-
flex: 0 0 0.8em;
482-
font-size: 1.5em;
483481
display: none;
484-
border: none;
485-
background: none;
486-
color: $fg-color;
487-
cursor: pointer;
488-
width: 0.8em;
489-
height: 0.8em;
490-
line-height: 0.8em;
491-
padding: 0;
492-
margin: 0 0.25em;
493482
}
494483

495484
.cellContents {

webview/src/experiments/util/buildDynamicColumns.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import Tooltip, {
1414
} from '../../shared/components/tooltip/Tooltip'
1515
import styles from '../components/table/styles.module.scss'
1616
import tooltipStyles from '../../shared/components/tooltip/styles.module.scss'
17-
import { CopyButton } from '../components/copyButton/CopyButton'
17+
import { CopyButton } from '../../shared/components/copyButton/CopyButton'
1818
import { OverflowHoverTooltip } from '../components/overflowHoverTooltip/OverflowHoverTooltip'
1919
const UndefinedCell = (
2020
<div className={styles.innerCell}>
@@ -63,7 +63,11 @@ const Cell: React.FC<Cell<Experiment, string | number>> = cell => {
6363
delay={[CELL_TOOLTIP_DELAY, 0]}
6464
>
6565
<div className={styles.innerCell}>
66-
<CopyButton value={stringValue} />
66+
<CopyButton
67+
value={stringValue}
68+
className={styles.copyButton}
69+
tooltip="Copy cell contents"
70+
/>
6771
<span className={styles.cellContents}>{displayValue}</span>
6872
</div>
6973
</Tooltip>

webview/src/plots/components/App.test.tsx

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import { act } from 'react-dom/test-utils'
3636
import { App } from './App'
3737
import { Plots } from './Plots'
3838
import { NewSectionBlock } from './templatePlots/TemplatePlots'
39-
import { CopyTooltip } from './ribbon/RibbonBlock'
4039
import { vsCodeApi } from '../../shared/api'
4140
import { createBubbledEvent, dragAndDrop, dragEnter } from '../../test/dragDrop'
4241
import { DragEnterDirection } from '../../shared/components/dragDrop/util'
@@ -1526,79 +1525,5 @@ describe('App', () => {
15261525
type: MessageFromWebviewType.REFRESH_REVISIONS
15271526
})
15281527
})
1529-
1530-
describe('Copy button', () => {
1531-
const mockWriteText = jest.fn()
1532-
Object.assign(navigator, {
1533-
clipboard: {
1534-
writeText: mockWriteText
1535-
}
1536-
})
1537-
1538-
beforeAll(() => {
1539-
jest.useFakeTimers()
1540-
})
1541-
1542-
afterAll(() => {
1543-
jest.useRealTimers()
1544-
})
1545-
1546-
it('should copy the experiment name when clicking the text', async () => {
1547-
mockWriteText.mockResolvedValueOnce('success')
1548-
1549-
renderAppWithData({
1550-
comparison: comparisonTableFixture,
1551-
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1552-
})
1553-
1554-
const mainNameButton = within(
1555-
screen.getByTestId('ribbon-main')
1556-
).getAllByRole('button')[0]
1557-
1558-
fireEvent.mouseEnter(mainNameButton, { bubbles: true })
1559-
fireEvent.click(mainNameButton)
1560-
1561-
expect(mockWriteText).toBeCalledWith('main')
1562-
await screen.findByText(CopyTooltip.COPIED)
1563-
})
1564-
1565-
it('should display that the experiment was copied when clicking the text', async () => {
1566-
mockWriteText.mockResolvedValueOnce('success')
1567-
1568-
renderAppWithData({
1569-
comparison: comparisonTableFixture,
1570-
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1571-
})
1572-
1573-
const mainNameButton = within(
1574-
screen.getByTestId('ribbon-main')
1575-
).getAllByRole('button')[0]
1576-
1577-
fireEvent.mouseEnter(mainNameButton, { bubbles: true })
1578-
fireEvent.click(mainNameButton)
1579-
1580-
expect(await screen.findByText(CopyTooltip.COPIED)).toBeInTheDocument()
1581-
})
1582-
1583-
it('should display copy again when hovering the text 2s after clicking the text', async () => {
1584-
mockWriteText.mockResolvedValueOnce('success')
1585-
1586-
renderAppWithData({
1587-
comparison: comparisonTableFixture,
1588-
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1589-
})
1590-
1591-
const mainNameButton = within(
1592-
screen.getByTestId('ribbon-main')
1593-
).getAllByRole('button')[0]
1594-
1595-
fireEvent.mouseEnter(mainNameButton, { bubbles: true })
1596-
fireEvent.click(mainNameButton)
1597-
1598-
jest.advanceTimersByTime(2001)
1599-
1600-
expect(await screen.findByText(CopyTooltip.NORMAL)).toBeInTheDocument()
1601-
})
1602-
})
16031528
})
16041529
})

webview/src/plots/components/ribbon/RibbonBlock.tsx

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { Revision } from 'dvc/src/plots/webview/contract'
2-
import React, { useRef, useState, useEffect } from 'react'
2+
import React from 'react'
33
import styles from './styles.module.scss'
44
import { AllIcons, Icon } from '../../../shared/components/Icon'
55
import Tooltip from '../../../shared/components/tooltip/Tooltip'
6+
import { CopyButton } from '../../../shared/components/copyButton/CopyButton'
67

78
interface RibbonBlockProps {
89
revision: Revision
@@ -18,26 +19,6 @@ export const RibbonBlock: React.FC<RibbonBlockProps> = ({
1819
revision,
1920
onClear
2021
}) => {
21-
const [copyTooltip, setCopyTooltip] = useState(CopyTooltip.NORMAL)
22-
const copyTooltipTimeout = useRef(0)
23-
24-
useEffect(() => {
25-
return () => {
26-
clearTimeout(copyTooltipTimeout.current)
27-
}
28-
}, [])
29-
30-
const copyExp = async (exp: string) => {
31-
try {
32-
await navigator.clipboard.writeText(exp)
33-
setCopyTooltip(CopyTooltip.COPIED)
34-
copyTooltipTimeout.current = window.setTimeout(() => {
35-
setCopyTooltip(CopyTooltip.NORMAL)
36-
}, 2000)
37-
} catch {
38-
setCopyTooltip(CopyTooltip.NORMAL)
39-
}
40-
}
4122
const exp = revision.group?.replace(/[[\]]/g, '') || revision.revision
4223

4324
return (
@@ -46,14 +27,15 @@ export const RibbonBlock: React.FC<RibbonBlockProps> = ({
4627
style={{ borderColor: revision.displayColor }}
4728
data-testid={`ribbon-${revision.id}`}
4829
>
49-
<Tooltip content={<>{copyTooltip}</>} hideOnClick={false} delay={500}>
50-
<button className={styles.label} onClick={() => copyExp(exp)}>
51-
<span>{exp}</span>
52-
{revision.group && (
53-
<span className={styles.subtitle}>{revision.revision}</span>
54-
)}
55-
</button>
56-
</Tooltip>
30+
<div className={styles.label}>
31+
<div className={styles.title}>
32+
<CopyButton value={exp} className={styles.copyButton} />
33+
{exp}
34+
</div>
35+
{revision.group && (
36+
<div className={styles.subtitle}>{revision.revision}</div>
37+
)}
38+
</div>
5739
<Tooltip content="Clear" placement="bottom" delay={500}>
5840
<button className={styles.clearButton} onClick={onClear}>
5941
<Icon icon={AllIcons.CLOSE} width={12} height={12} />

webview/src/plots/components/ribbon/styles.module.scss

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
@import '../../../shared/variables.scss';
22

3+
.copyButton {
4+
display: none;
5+
}
6+
37
.block {
48
border-left-style: solid;
59
border-left-width: 3px;
@@ -16,6 +20,11 @@
1620
fill: $fg-color;
1721
display: block;
1822
}
23+
24+
&:hover .copyButton {
25+
display: inline;
26+
font-size: 0.8125rem;
27+
}
1928
}
2029

2130
.label {
@@ -24,11 +33,15 @@
2433
border: none;
2534
text-align: left;
2635
color: $fg-color;
36+
display: flex;
37+
flex-direction: column;
38+
justify-content: center;
2739
min-width: max-content;
40+
}
2841

29-
span {
30-
display: block;
31-
}
42+
.title {
43+
display: inline-flex;
44+
align-items: center;
3245
}
3346

3447
.subtitle {

webview/src/experiments/components/copyButton/CopyButton.test.tsx renamed to webview/src/shared/components/copyButton/CopyButton.test.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('CopyButton', () => {
3535

3636
it('should call writeText with the value prop and show the success icon for a second when writeText resolves', async () => {
3737
mockWriteText.mockResolvedValueOnce(undefined)
38-
render(<CopyButton value={exampleCopyText} />)
38+
render(<CopyButton value={exampleCopyText} tooltip={defaultStateTitle} />)
3939
const copyButtonElement = screen.getByTitle(defaultStateTitle)
4040

4141
fireEvent.click(copyButtonElement, {
@@ -53,7 +53,7 @@ describe('CopyButton', () => {
5353

5454
it('should call writeText with the value prop and show the failure icon for a second when writeText rejects', async () => {
5555
mockWriteText.mockRejectedValueOnce(new Error('Copying is not allowed!'))
56-
render(<CopyButton value={exampleCopyText} />)
56+
render(<CopyButton value={exampleCopyText} tooltip={defaultStateTitle} />)
5757
const copyButtonElement = screen.getByTitle(defaultStateTitle)
5858

5959
fireEvent.click(copyButtonElement, {
@@ -73,7 +73,7 @@ describe('CopyButton', () => {
7373

7474
it('should restart the state reset timer if clicked while in the success state', async () => {
7575
mockWriteText.mockResolvedValueOnce(undefined)
76-
render(<CopyButton value={exampleCopyText} />)
76+
render(<CopyButton value={exampleCopyText} tooltip={defaultStateTitle} />)
7777
const copyButtonElement = screen.getByTitle(defaultStateTitle)
7878

7979
// Click once

webview/src/experiments/components/copyButton/CopyButton.tsx renamed to webview/src/shared/components/copyButton/CopyButton.tsx

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import React, { FC, useEffect, useRef, useState } from 'react'
2-
import CopyIcon from '../../../shared/components/icons/Copy'
3-
import CheckIcon from '../../../shared/components/icons/Check'
4-
import styles from '../table/styles.module.scss'
2+
import cx from 'classnames'
3+
import styles from './styles.module.scss'
4+
import CopyIcon from '../icons/Copy'
5+
import CheckIcon from '../icons/Check'
56

67
const enum CopyButtonState {
78
DEFAULT,
@@ -17,13 +18,17 @@ const copyIconComponents: Record<CopyButtonState, FC> = {
1718
[CopyButtonState.FAILURE]: FailureIcon
1819
}
1920

20-
const copyIconTitles: Record<CopyButtonState, string> = {
21-
[CopyButtonState.DEFAULT]: 'Copy cell contents',
22-
[CopyButtonState.SUCCESS]: 'Copy successful',
23-
[CopyButtonState.FAILURE]: 'Copy failed'
24-
}
21+
export const CopyButton: React.FC<{
22+
value: string
23+
tooltip?: string
24+
className?: string
25+
}> = ({ value, tooltip, className }) => {
26+
const copyIconTitles: Record<CopyButtonState, string> = {
27+
[CopyButtonState.DEFAULT]: tooltip || 'Copy',
28+
[CopyButtonState.SUCCESS]: 'Copy successful',
29+
[CopyButtonState.FAILURE]: 'Copy failed'
30+
}
2531

26-
export const CopyButton: React.FC<{ value: string }> = ({ value }) => {
2732
const timer = useRef<number>()
2833
const [state, setState] = useState<CopyButtonState>(CopyButtonState.DEFAULT)
2934
const IconComponent = copyIconComponents[state]
@@ -38,7 +43,7 @@ export const CopyButton: React.FC<{ value: string }> = ({ value }) => {
3843
return (
3944
<button
4045
title={copyIconTitles[state]}
41-
className={styles.copyButton}
46+
className={cx(styles.button, className)}
4247
onClick={() => {
4348
navigator.clipboard
4449
.writeText(value)
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
@import '../../variables.scss';
2+
3+
.button {
4+
flex: 0 0 0.8em;
5+
font-size: 1.5em;
6+
display: none;
7+
border: none;
8+
background: none;
9+
color: $fg-color;
10+
cursor: pointer;
11+
width: 0.8em;
12+
height: 0.8em;
13+
line-height: 0.8em;
14+
padding: 0;
15+
margin: 0 0.25em;
16+
}

0 commit comments

Comments
 (0)