Skip to content

Commit d28756f

Browse files
julieg18julieg18
authored
Add "Add Top-level Plot" command to plots webview (#4664)
1 parent fa54bc2 commit d28756f

File tree

11 files changed

+90
-26
lines changed

11 files changed

+90
-26
lines changed

‎extension/src/pipeline/register.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { WorkspacePipeline } from './workspace'
22
import { RegisteredCommands } from '../commands/external'
33
import { InternalCommands } from '../commands/internal'
4+
import { Context, getDvcRootFromContext } from '../vscode/context'
45

56
export const registerPipelineCommands = (
67
pipelines: WorkspacePipeline,
@@ -13,6 +14,7 @@ export const registerPipelineCommands = (
1314

1415
internalCommands.registerExternalCommand(
1516
RegisteredCommands.PIPELINE_ADD_PLOT,
16-
() => pipelines.addTopLevelPlot()
17+
(context: Context) =>
18+
pipelines.addTopLevelPlot(getDvcRootFromContext(context))
1719
)
1820
}

‎extension/src/pipeline/workspace.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ export class WorkspacePipeline extends BaseWorkspace<Pipeline> {
6363
)
6464
}
6565

66-
public async addTopLevelPlot() {
67-
const cwd = await this.getCwd()
66+
public async addTopLevelPlot(overrideRoot?: string) {
67+
const cwd = overrideRoot || (await this.getCwd())
6868

6969
if (!cwd) {
7070
return

‎extension/src/plots/webview/messages.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ export class WebviewMessages {
7676
RegisteredCommands.PLOTS_CUSTOM_ADD,
7777
this.dvcRoot
7878
)
79+
case MessageFromWebviewType.ADD_PIPELINE_PLOT:
80+
return commands.executeCommand(
81+
RegisteredCommands.PIPELINE_ADD_PLOT,
82+
this.dvcRoot
83+
)
7984
case MessageFromWebviewType.EXPORT_PLOT_DATA_AS_CSV:
8085
return this.exportPlotDataAsCsv(message.payload)
8186
case MessageFromWebviewType.EXPORT_PLOT_DATA_AS_TSV:

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,24 @@ suite('Plots Test Suite', () => {
12091209
)
12101210
})
12111211

1212+
it('should handle an add pipeline plot message from the webview', async () => {
1213+
const { mockMessageReceived } = await buildPlotsWebview({
1214+
disposer: disposable,
1215+
plotsDiff: plotsDiffFixture
1216+
})
1217+
1218+
const executeCommandSpy = spy(commands, 'executeCommand')
1219+
1220+
mockMessageReceived.fire({
1221+
type: MessageFromWebviewType.ADD_PIPELINE_PLOT
1222+
})
1223+
1224+
expect(executeCommandSpy).to.be.calledWithExactly(
1225+
RegisteredCommands.PIPELINE_ADD_PLOT,
1226+
dvcDemoPath
1227+
)
1228+
})
1229+
12121230
it('should handle the CLI throwing an error', async () => {
12131231
const { data, errorsModel, mockPlotsDiff, plots, plotsModel } =
12141232
await buildPlots({ disposer: disposable, plotsDiff: plotsDiffFixture })

‎extension/src/webview/contract.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export enum MessageFromWebviewType {
1717
APPLY_EXPERIMENT_TO_WORKSPACE = 'apply-experiment-to-workspace',
1818
ADD_STARRED_EXPERIMENT_FILTER = 'add-starred-experiment-filter',
1919
ADD_CUSTOM_PLOT = 'add-custom-plot',
20+
ADD_PIPELINE_PLOT = 'add-pipeline-plot',
2021
CREATE_BRANCH_FROM_EXPERIMENT = 'create-branch-from-experiment',
2122
COPY_TO_CLIPBOARD = 'copy-to-clipboard',
2223
COPY_STUDIO_LINK = 'copy-studio-link',
@@ -110,6 +111,9 @@ export type MessageFromWebview =
110111
| {
111112
type: MessageFromWebviewType.ADD_CUSTOM_PLOT
112113
}
114+
| {
115+
type: MessageFromWebviewType.ADD_PIPELINE_PLOT
116+
}
113117
| {
114118
type: MessageFromWebviewType.COPY_TO_CLIPBOARD
115119
payload: string

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

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -336,18 +336,18 @@ describe('App', () => {
336336
expect(loading).toHaveLength(3)
337337
})
338338

339-
it('should render only get started (buttons: add plots, add experiments, add custom plots) when there are some selected exps, all unselected plots, and no custom plots', async () => {
339+
it('should render only get started (buttons: select plots, add experiments, add custom plots) when there are some selected exps, all unselected plots, and no custom plots', async () => {
340340
renderAppWithOptionalData({
341341
hasPlots: true,
342342
hasUnselectedPlots: true,
343343
selectedRevisions: [{} as Revision]
344344
})
345345
const addExperimentsButton = await screen.findByText('Add Experiments')
346-
const addPlotsButton = await screen.findByText('Add Plots')
346+
const selectPlotsButton = await screen.findByText('Select Plots')
347347
const addCustomPlotsButton = await screen.findByText('Add Custom Plot')
348348

349349
expect(addExperimentsButton).toBeInTheDocument()
350-
expect(addPlotsButton).toBeInTheDocument()
350+
expect(selectPlotsButton).toBeInTheDocument()
351351
expect(addCustomPlotsButton).toBeInTheDocument()
352352
expect(screen.queryByTestId('section-container')).not.toBeInTheDocument()
353353

@@ -361,7 +361,7 @@ describe('App', () => {
361361

362362
mockPostMessage.mockReset()
363363

364-
fireEvent.click(addPlotsButton)
364+
fireEvent.click(selectPlotsButton)
365365

366366
expect(mockPostMessage).toHaveBeenCalledWith({
367367
type: MessageFromWebviewType.SELECT_PLOTS
@@ -427,20 +427,20 @@ describe('App', () => {
427427
mockPostMessage.mockReset()
428428
})
429429

430-
it('should render get started (buttons: add plots, add experiments) and custom section when there are some selected exps, all unselected plots, and added custom plots', async () => {
430+
it('should render get started (buttons: select plots, add experiments) and custom section when there are some selected exps, all unselected plots, and added custom plots', async () => {
431431
renderAppWithOptionalData({
432432
custom: customPlotsFixture,
433433
hasPlots: true,
434434
hasUnselectedPlots: true,
435435
selectedRevisions: [{} as Revision]
436436
})
437437
const addExperimentsButton = await screen.findByText('Add Experiments')
438-
const addPlotsButton = await screen.findByText('Add Plots')
438+
const selectPlotsButton = await screen.findByText('Select Plots')
439439
const addCustomPlotsButton = screen.queryByText('Add Custom Plot')
440440
const customSection = await screen.findByTestId('section-container')
441441

442442
expect(addExperimentsButton).toBeInTheDocument()
443-
expect(addPlotsButton).toBeInTheDocument()
443+
expect(selectPlotsButton).toBeInTheDocument()
444444
expect(addCustomPlotsButton).not.toBeInTheDocument()
445445
expect(customSection).toBeInTheDocument()
446446

@@ -454,29 +454,29 @@ describe('App', () => {
454454

455455
mockPostMessage.mockReset()
456456

457-
fireEvent.click(addPlotsButton)
457+
fireEvent.click(selectPlotsButton)
458458

459459
expect(mockPostMessage).toHaveBeenCalledWith({
460460
type: MessageFromWebviewType.SELECT_PLOTS
461461
})
462462
mockPostMessage.mockReset()
463463
})
464464

465-
it('should render only get started (buttons: add experiments, add custom plots) when there are no selected exps and no custom plots', async () => {
465+
it('should render only get started (buttons: add experiments, add custom plots, add plots) when there are no selected exps and no custom plots', async () => {
466466
renderAppWithOptionalData({
467467
custom: null,
468468
hasPlots: true,
469469
hasUnselectedPlots: false,
470470
selectedRevisions: undefined
471471
})
472472
const addExperimentsButton = await screen.findByText('Add Experiments')
473-
const addPlotsButton = screen.queryByText('Add Plots')
473+
const addPlotsButton = await screen.findByText('Add Plot')
474474
const addCustomPlotsButton = await screen.findByText('Add Custom Plot')
475475
const customSection = screen.queryByTestId('section-container')
476476

477477
expect(addExperimentsButton).toBeInTheDocument()
478478
expect(addCustomPlotsButton).toBeInTheDocument()
479-
expect(addPlotsButton).not.toBeInTheDocument()
479+
expect(addPlotsButton).toBeInTheDocument()
480480
expect(customSection).not.toBeInTheDocument()
481481

482482
mockPostMessage.mockReset()
@@ -492,23 +492,29 @@ describe('App', () => {
492492
type: MessageFromWebviewType.ADD_CUSTOM_PLOT
493493
})
494494
mockPostMessage.mockReset()
495+
496+
fireEvent.click(addPlotsButton)
497+
498+
expect(mockPostMessage).toHaveBeenCalledWith({
499+
type: MessageFromWebviewType.ADD_PIPELINE_PLOT
500+
})
495501
})
496502

497-
it('should render get started (buttons: add experiments) and custom section when there are no selected exps and added custom plots', async () => {
503+
it('should render get started (buttons: add experiments, add plots) and custom section when there are no selected exps and added custom plots', async () => {
498504
renderAppWithOptionalData({
499505
custom: customPlotsFixture,
500506
hasPlots: true,
501507
hasUnselectedPlots: false,
502508
selectedRevisions: undefined
503509
})
504510
const addExperimentsButton = await screen.findByText('Add Experiments')
505-
const addPlotsButton = screen.queryByText('Add Plots')
511+
const addPlotsButton = await screen.findByText('Add Plot')
506512
const addCustomPlotsButton = screen.queryByText('Add Custom Plot')
507513
const customSection = await screen.findByTestId('section-container')
508514

509515
expect(addExperimentsButton).toBeInTheDocument()
510516
expect(addCustomPlotsButton).not.toBeInTheDocument()
511-
expect(addPlotsButton).not.toBeInTheDocument()
517+
expect(addPlotsButton).toBeInTheDocument()
512518
expect(customSection).toBeInTheDocument()
513519

514520
mockPostMessage.mockReset()
@@ -517,6 +523,13 @@ describe('App', () => {
517523
expect(mockPostMessage).toHaveBeenCalledWith({
518524
type: MessageFromWebviewType.SELECT_EXPERIMENTS
519525
})
526+
527+
mockPostMessage.mockReset()
528+
529+
fireEvent.click(addPlotsButton)
530+
expect(mockPostMessage).toHaveBeenCalledWith({
531+
type: MessageFromWebviewType.ADD_PIPELINE_PLOT
532+
})
520533
})
521534

522535
it('should render custom with "No Plots to Display" message when there is no custom plots data', () => {
@@ -763,16 +776,14 @@ describe('App', () => {
763776

764777
const customSection = screen.getAllByTestId('section-container')[2]
765778

766-
expect(
767-
within(customSection).getByLabelText('Add Plots')
768-
).toBeInTheDocument()
779+
expect(within(customSection).getByLabelText('Add Plot')).toBeInTheDocument()
769780

770781
sendSetDataMessage({
771782
custom: { ...customPlotsFixture, enablePlotCreation: false }
772783
})
773784

774785
expect(
775-
within(customSection).queryByLabelText('Add Plots')
786+
within(customSection).queryByLabelText('Add Plot')
776787
).not.toBeInTheDocument()
777788
})
778789

‎webview/src/plots/components/PlotsContainer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export const PlotsContainer: React.FC<PlotsContainerProps> = ({
7474
menuItems.unshift({
7575
icon: Add,
7676
onClick: addPlotsButton.onClick,
77-
tooltip: 'Add Plots'
77+
tooltip: 'Add Plot'
7878
})
7979
}
8080

‎webview/src/plots/components/emptyState/AddPlots.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react'
22
import {
33
addCustomPlot,
4+
addPipelinePlot,
45
selectPlots,
56
selectRevisions
67
} from '../../util/messages'
@@ -19,12 +20,19 @@ export const AddPlots: React.FC<AddPlotsProps> = ({
1920
<p>No Plots to Display</p>
2021
<div>
2122
<StartButton onClick={selectRevisions} text="Add Experiments" />
22-
{hasUnselectedPlots && (
23+
{hasUnselectedPlots ? (
2324
<StartButton
2425
isNested={true}
2526
appearance="secondary"
2627
onClick={selectPlots}
27-
text="Add Plots"
28+
text="Select Plots"
29+
/>
30+
) : (
31+
<StartButton
32+
isNested={true}
33+
appearance="secondary"
34+
onClick={addPipelinePlot}
35+
text="Add Plot"
2836
/>
2937
)}
3038
{!hasCustomPlots && (

‎webview/src/plots/components/emptyState/Welcome.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
import React from 'react'
22
import { StartButton } from '../../../shared/components/button/StartButton'
3-
import { selectRevisions } from '../../util/messages'
3+
import { addPipelinePlot, selectRevisions } from '../../util/messages'
44

55
export const Welcome: React.FC = () => (
66
<div>
77
<p>No Plots Detected.</p>
8-
<StartButton onClick={selectRevisions} text="Add Experiments" />
8+
<div>
9+
<StartButton onClick={addPipelinePlot} text="Add Plot" />
10+
<StartButton
11+
appearance="secondary"
12+
onClick={selectRevisions}
13+
text="Add Experiments"
14+
isNested={true}
15+
/>
16+
</div>
917
<p>
1018
Learn how to{' '}
1119
<a href="https://dvc.org/doc/user-guide/experiment-management/visualizing-plots">

‎webview/src/plots/components/templatePlots/TemplatePlotsWrapper.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { TemplatePlots } from './TemplatePlots'
55
import { changeSize } from './templatePlotsSlice'
66
import { PlotsContainer } from '../PlotsContainer'
77
import { PlotsState } from '../../store'
8+
import { addPipelinePlot } from '../../util/messages'
89

910
export const TemplatePlotsWrapper: React.FC = () => {
1011
const { nbItemsPerRow, isCollapsed, height, hasItems } = useSelector(
@@ -20,6 +21,7 @@ export const TemplatePlotsWrapper: React.FC = () => {
2021
changeSize={changeSize}
2122
hasItems={hasItems}
2223
height={height}
24+
addPlotsButton={{ onClick: addPipelinePlot }}
2325
>
2426
<TemplatePlots />
2527
</PlotsContainer>

0 commit comments

Comments
 (0)