Skip to content

Commit c8c3a22

Browse files
authored
Add plots ribbon to show and remove experiments (#1785)
* Add plots ribbon * Fix tests and event name * Add tests * Add react-virtualized * Add extension test * Apply review comments
1 parent 8de768e commit c8c3a22

File tree

16 files changed

+365
-51
lines changed

16 files changed

+365
-51
lines changed

extension/src/plots/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ export class Plots extends BaseRepository<TPlotsData> {
268268
return this.selectExperimentsFromWebview()
269269
case MessageFromWebviewType.REFRESH_REVISION:
270270
return this.attemptToRefreshData(message.payload)
271+
case MessageFromWebviewType.TOGGLE_EXPERIMENT:
272+
return this.setExperimentStatus(message.payload)
271273
default:
272274
Logger.error(`Unexpected message: ${JSON.stringify(message)}`)
273275
}
@@ -353,6 +355,15 @@ export class Plots extends BaseRepository<TPlotsData> {
353355
)
354356
}
355357

358+
private setExperimentStatus(id: string) {
359+
this.experiments?.toggleExperimentStatus(id)
360+
sendTelemetryEvent(
361+
EventName.VIEWS_PLOTS_EXPERIMENT_TOGGLE,
362+
undefined,
363+
undefined
364+
)
365+
}
366+
356367
private attemptToRefreshData(revision: string) {
357368
Toast.infoWithOptions(`Attempting to refresh ${revision} plots data.`)
358369
this.plots?.setupManualRefresh(revision)

extension/src/plots/model/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
import {
1515
CheckpointPlotData,
1616
ComparisonPlots,
17-
ComparisonRevision,
17+
Revision,
1818
ComparisonRevisionData,
1919
DEFAULT_SECTION_COLLAPSED,
2020
DEFAULT_SECTION_NAMES,
@@ -187,13 +187,14 @@ export class PlotsModel extends ModelWithPersistence {
187187
}
188188

189189
public getSelectedRevisionDetails() {
190-
return reorderObjectList<ComparisonRevision>(
190+
return reorderObjectList<Revision>(
191191
this.comparisonOrder,
192192
this.experiments
193193
.getSelectedRevisions()
194-
.map(({ label: revision, displayColor, logicalGroupName }) => ({
194+
.map(({ label: revision, displayColor, logicalGroupName, id }) => ({
195195
displayColor,
196196
group: logicalGroupName,
197+
id,
197198
revision
198199
})),
199200
'revision'

extension/src/plots/vega/util.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
RepeatMapping
1818
} from 'vega-lite/build/src/spec/repeat'
1919
import { TopLevelUnitSpec } from 'vega-lite/build/src/spec/unit'
20-
import { ColorScale, ComparisonRevision } from '../webview/contract'
20+
import { ColorScale, Revision } from '../webview/contract'
2121

2222
const COMMIT_FIELD = 'rev'
2323

@@ -84,7 +84,7 @@ export const isMultiViewByCommitPlot = (
8484
): boolean => !template || getFacetField(template) === COMMIT_FIELD
8585

8686
export const getColorScale = (
87-
revisions: ComparisonRevision[]
87+
revisions: Revision[]
8888
): ColorScale | undefined => {
8989
const acc: ColorScale = { domain: [], range: [] }
9090

extension/src/plots/webview/contract.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,15 @@ export type ComparisonPlots = {
4343
revisions: ComparisonRevisionData
4444
}[]
4545

46-
export type ComparisonRevision = {
46+
export type Revision = {
47+
id?: string
4748
revision: string
4849
group?: string
4950
displayColor: Color
5051
}
5152

5253
export interface PlotsComparisonData {
53-
revisions: ComparisonRevision[]
54+
revisions: Revision[]
5455
plots: ComparisonPlots
5556
sectionName: string
5657
size: PlotSize

extension/src/telemetry/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const EventName = Object.assign(
4848

4949
VIEWS_PLOTS_CLOSED: 'views.plots.closed',
5050
VIEWS_PLOTS_CREATED: 'views.plots.created',
51+
VIEWS_PLOTS_EXPERIMENT_TOGGLE: 'views.plots.toggleExperimentStatus',
5152
VIEWS_PLOTS_FOCUS_CHANGED: 'views.plots.focusChanged',
5253
VIEWS_PLOTS_MANUAL_REFRESH: 'views.plots.manualRefresh',
5354
VIEWS_PLOTS_METRICS_SELECTED: 'views.plots.metricsSelected',
@@ -207,6 +208,7 @@ export interface IEventNamePropertyMapping {
207208
[EventName.VIEWS_PLOTS_SECTION_TOGGLE]: Partial<SectionCollapsed>
208209
[EventName.VIEWS_PLOTS_SELECT_EXPERIMENTS]: undefined
209210
[EventName.VIEWS_PLOTS_SELECT_PLOTS]: undefined
211+
[EventName.VIEWS_PLOTS_EXPERIMENT_TOGGLE]: undefined
210212
[EventName.VIEWS_REORDER_PLOTS_METRICS]: undefined
211213
[EventName.VIEWS_REORDER_PLOTS_TEMPLATES]: undefined
212214

extension/src/test/fixtures/plotsDiff/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,22 +546,26 @@ export const getComparisonWebviewMessage = (
546546
plots: plotAcc,
547547
revisions: [
548548
{
549+
id: 'workspace',
549550
revision: 'workspace',
550551
displayColor: workspace,
551552
group: undefined
552553
},
553-
{ revision: 'main', displayColor: main, group: undefined },
554+
{ id: 'main', revision: 'main', displayColor: main, group: undefined },
554555
{
556+
id: 'exp-e7a67',
555557
revision: '4fb124a',
556558
displayColor: _4fb124a,
557559
group: '[exp-e7a67]'
558560
},
559561
{
562+
id: 'test-branch',
560563
revision: '42b8736',
561564
displayColor: _42b8735,
562565
group: '[test-branch]'
563566
},
564567
{
568+
id: 'exp-83425',
565569
revision: '1ba7bcd',
566570
displayColor: _1ba7bcd,
567571
group: '[exp-83425]'

extension/src/test/suite/experiments/model/tree.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,36 +310,43 @@ suite('Experiments Tree Test Suite', () => {
310310
{
311311
displayColor: colors[0],
312312
group: undefined,
313+
id: 'workspace',
313314
revision: 'workspace'
314315
},
315316
{
316317
displayColor: colors[2],
317318
group: '[exp-e7a67]',
319+
id: 'exp-e7a67',
318320
revision: '4fb124a'
319321
},
320322
{
321323
displayColor: colors[3],
322324
group: '[test-branch]',
325+
id: 'test-branch',
323326
revision: '42b8736'
324327
},
325328
{
326329
displayColor: colors[1],
327330
group: '[exp-e7a67]',
331+
id: 'd1343a87c6ee4a2e82d19525964d2fb2cb6756c9',
328332
revision: 'd1343a8'
329333
},
330334
{
331335
displayColor: colors[4],
332336
group: '[exp-e7a67]',
337+
id: '1ee5f2ecb0fa4d83cbf614386536344cf894dd53',
333338
revision: '1ee5f2e'
334339
},
335340
{
336341
displayColor: colors[5],
337342
group: '[test-branch]',
343+
id: '217312476f8854dda1865450b737eb6bc7a3ba1b',
338344
revision: '2173124'
339345
},
340346
{
341347
displayColor: colors[6],
342348
group: '[test-branch]',
349+
id: '9523bde67538cf31230efaff2dbc47d38a944ab5',
343350
revision: '9523bde'
344351
}
345352
])

extension/src/test/suite/plots/index.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,5 +593,35 @@ suite('Plots Test Suite', () => {
593593
expect(webview.isActive()).to.be.true
594594
expect(webview.isVisible()).to.be.true
595595
}).timeout(WEBVIEW_TEST_TIMEOUT)
596+
597+
it('should handle a toggle experiment message from the webview', async () => {
598+
const { plots, experiments } = await buildPlots(
599+
disposable,
600+
plotsDiffFixture
601+
)
602+
603+
const mockSelectExperiments = stub(
604+
experiments,
605+
'toggleExperimentStatus'
606+
).resolves(undefined)
607+
608+
const webview = await plots.showWebview()
609+
610+
const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
611+
const mockMessageReceived = getMessageReceivedEmitter(webview)
612+
613+
mockMessageReceived.fire({
614+
payload: 'main',
615+
type: MessageFromWebviewType.TOGGLE_EXPERIMENT
616+
})
617+
618+
expect(mockSelectExperiments).to.be.calledOnce
619+
expect(mockSendTelemetryEvent).to.be.calledOnce
620+
expect(mockSendTelemetryEvent).to.be.calledWithExactly(
621+
EventName.VIEWS_PLOTS_EXPERIMENT_TOGGLE,
622+
undefined,
623+
undefined
624+
)
625+
}).timeout(WEBVIEW_TEST_TIMEOUT)
596626
})
597627
})

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

Lines changed: 109 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ 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'
3940
import { vsCodeApi } from '../../shared/api'
4041
import { createBubbledEvent, dragAndDrop, dragEnter } from '../../test/dragDrop'
4142
import { DragEnterDirection } from '../../shared/components/dragDrop/util'
@@ -353,7 +354,9 @@ describe('App', () => {
353354
cancelable: true
354355
})
355356

356-
const workspaceHeader = screen.queryByText('workspace')
357+
const comparisonTable = screen.getByRole('table')
358+
359+
const workspaceHeader = within(comparisonTable).queryByText('workspace')
357360
expect(workspaceHeader).toBeInTheDocument()
358361

359362
const [, pickerButton] = screen.queryAllByTestId('icon-menu-item')
@@ -378,8 +381,7 @@ describe('App', () => {
378381
cancelable: true
379382
})
380383

381-
const table = screen.getByRole('table')
382-
expect(within(table).getByText('workspace')).toBeInTheDocument()
384+
expect(within(comparisonTable).getByText('workspace')).toBeInTheDocument()
383385
})
384386

385387
it('should show the newest revision in the comparision table even if some revisions were filtered out', async () => {
@@ -432,7 +434,7 @@ describe('App', () => {
432434
)
433435

434436
const workspaceHeader = screen.queryAllByText(newRevision)
435-
expect(workspaceHeader.length).toBe(2) // One in the table and one in the menu
437+
expect(workspaceHeader.length).toBe(3) // One in the table, one in the menu, and one in the ribbon
436438
})
437439

438440
it('should send a message to the extension with the selected metrics when toggling the visibility of a plot', async () => {
@@ -1541,4 +1543,107 @@ describe('App', () => {
15411543
expect(contextMenuEvent.defaultPrevented).toBe(true)
15421544
})
15431545
})
1546+
1547+
describe('Ribbon', () => {
1548+
it('should show the revisions at the top', () => {
1549+
renderAppWithData({
1550+
comparison: comparisonTableFixture,
1551+
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1552+
})
1553+
const ribbon = screen.getByTestId('ribbon')
1554+
1555+
const revisions = within(ribbon)
1556+
.getAllByRole('listitem')
1557+
.map(item => item.textContent)
1558+
expect(revisions).toStrictEqual(
1559+
comparisonTableFixture.revisions.map(rev =>
1560+
rev.group ? rev.group.slice(1, -1) + rev.revision : rev.revision
1561+
)
1562+
)
1563+
})
1564+
1565+
it('should send a message with the revision to be removed when clicking the clear button', () => {
1566+
renderAppWithData({
1567+
comparison: comparisonTableFixture,
1568+
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1569+
})
1570+
1571+
const mainClearButton = within(
1572+
screen.getByTestId('ribbon-main')
1573+
).getAllByRole('button')[1]
1574+
1575+
fireEvent.click(mainClearButton)
1576+
1577+
expect(mockPostMessage).toBeCalledWith({
1578+
payload: 'main',
1579+
type: MessageFromWebviewType.TOGGLE_EXPERIMENT
1580+
})
1581+
})
1582+
1583+
describe('Copy button', () => {
1584+
const mockWriteText = jest.fn()
1585+
Object.assign(navigator, {
1586+
clipboard: {
1587+
writeText: mockWriteText
1588+
}
1589+
})
1590+
1591+
it('should copy the experiment name when clicking the text', () => {
1592+
renderAppWithData({
1593+
comparison: comparisonTableFixture,
1594+
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1595+
})
1596+
1597+
const mainNameButton = within(
1598+
screen.getByTestId('ribbon-main')
1599+
).getAllByRole('button')[0]
1600+
1601+
fireEvent.click(mainNameButton)
1602+
1603+
expect(mockWriteText).toBeCalledWith('main')
1604+
})
1605+
1606+
it('should display that the experiment was copied when clicking the text', async () => {
1607+
jest.useFakeTimers()
1608+
1609+
mockWriteText.mockResolvedValueOnce('success')
1610+
1611+
renderAppWithData({
1612+
comparison: comparisonTableFixture,
1613+
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1614+
})
1615+
1616+
const mainNameButton = within(
1617+
screen.getByTestId('ribbon-main')
1618+
).getAllByRole('button')[0]
1619+
1620+
fireEvent.mouseEnter(mainNameButton, { bubbles: true })
1621+
fireEvent.click(mainNameButton)
1622+
1623+
expect(await screen.findByText(CopyTooltip.COPIED)).toBeInTheDocument()
1624+
jest.useRealTimers()
1625+
})
1626+
1627+
it('should display copy again when hovering the text 2s after clicking the text', () => {
1628+
jest.useFakeTimers()
1629+
1630+
renderAppWithData({
1631+
comparison: comparisonTableFixture,
1632+
sectionCollapsed: DEFAULT_SECTION_COLLAPSED
1633+
})
1634+
1635+
const mainNameButton = within(
1636+
screen.getByTestId('ribbon-main')
1637+
).getAllByRole('button')[0]
1638+
1639+
fireEvent.click(mainNameButton)
1640+
fireEvent.mouseEnter(mainNameButton, { bubbles: true })
1641+
1642+
jest.advanceTimersByTime(2001)
1643+
1644+
expect(screen.getByText(CopyTooltip.NORMAL)).toBeInTheDocument()
1645+
jest.useRealTimers()
1646+
})
1647+
})
1648+
})
15441649
})

0 commit comments

Comments
 (0)