Skip to content

Commit ecdb189

Browse files
authored
Merge pull request #1906 from iterative/multi-select-ctx-menu-experiments-table
Add Multi-Select context menu to Experiments Table
2 parents c097f5d + 3ed80d8 commit ecdb189

File tree

16 files changed

+448
-93
lines changed

16 files changed

+448
-93
lines changed

extension/src/experiments/index.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -624,8 +624,11 @@ export class Experiments extends BaseRepository<TableData> {
624624
return this.runCommand(AvailableCommands.EXPERIMENT_APPLY, experimentId)
625625
}
626626

627-
private removeExperiment(experimentId: string) {
628-
return this.runCommand(AvailableCommands.EXPERIMENT_REMOVE, experimentId)
627+
private removeExperiment(experimentId: string | string[]) {
628+
return this.runCommand(
629+
AvailableCommands.EXPERIMENT_REMOVE,
630+
...[experimentId].flat()
631+
)
629632
}
630633

631634
private async checkAutoApplyFilters(...filterIdsToRemove: string[]) {

extension/src/webview/contract.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export type MessageFromWebview =
9797
}
9898
| {
9999
type: MessageFromWebviewType.REMOVE_EXPERIMENT
100-
payload: string
100+
payload: string | string[]
101101
}
102102
| {
103103
type: MessageFromWebviewType.SORT_COLUMN

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,134 @@ describe('App', () => {
671671
})
672672
})
673673

674+
describe('Row Context Menu', () => {
675+
it('should be available when there is data and no running experiments', async () => {
676+
render(<App />)
677+
678+
fireEvent(
679+
window,
680+
new MessageEvent('message', {
681+
data: {
682+
data: {
683+
...tableDataFixture,
684+
hasRunningExperiment: false
685+
},
686+
type: MessageToWebviewType.SET_DATA
687+
}
688+
})
689+
)
690+
691+
const target = screen.getByTestId('workspace-row')
692+
fireEvent.contextMenu(target, { bubbles: true })
693+
694+
const menu = await screen.findByTestId('messages-menu')
695+
expect(menu).toBeDefined()
696+
})
697+
698+
it('should present the correct options for the workspace row with no checkpoints', async () => {
699+
render(<App />)
700+
701+
fireEvent(
702+
window,
703+
new MessageEvent('message', {
704+
data: {
705+
data: deeplyNestedTableDataFixture,
706+
type: MessageToWebviewType.SET_DATA
707+
}
708+
})
709+
)
710+
711+
const target = screen.getByTestId('workspace-row')
712+
fireEvent.contextMenu(target, { bubbles: true })
713+
714+
const menuitems = await screen.findAllByRole('menuitem')
715+
const itemLabels = menuitems.map(item => item.textContent)
716+
expect(itemLabels).toStrictEqual(['Modify and Run', 'Modify and Queue'])
717+
})
718+
719+
it('should present the correct options for the main row with checkpoints', async () => {
720+
render(<App />)
721+
722+
fireEvent(
723+
window,
724+
new MessageEvent('message', {
725+
data: {
726+
data: {
727+
...tableDataFixture,
728+
hasRunningExperiment: false
729+
},
730+
type: MessageToWebviewType.SET_DATA
731+
}
732+
})
733+
)
734+
735+
const target = screen.getByText('main')
736+
fireEvent.contextMenu(target, { bubbles: true })
737+
738+
const menuitems = await screen.findAllByRole('menuitem')
739+
const itemLabels = menuitems.map(item => item.textContent)
740+
expect(itemLabels).toStrictEqual([
741+
'Modify and Resume',
742+
'Modify, Reset and Run',
743+
'Modify and Queue'
744+
])
745+
})
746+
747+
it('should present the Remove experiment option for the checkpoint tips', async () => {
748+
render(<App />)
749+
750+
fireEvent(
751+
window,
752+
new MessageEvent('message', {
753+
data: {
754+
data: {
755+
...tableDataFixture,
756+
hasRunningExperiment: false
757+
},
758+
type: MessageToWebviewType.SET_DATA
759+
}
760+
})
761+
)
762+
763+
const target = screen.getByText('4fb124a')
764+
fireEvent.contextMenu(target, { bubbles: true })
765+
766+
const menuitems = await screen.findAllByRole('menuitem')
767+
const itemLabels = menuitems.map(item => item.textContent)
768+
expect(itemLabels).toContain('Remove')
769+
})
770+
771+
it('should present the Remove option if multiple checkpoint tip rows are selected', async () => {
772+
render(<App />)
773+
774+
fireEvent(
775+
window,
776+
new MessageEvent('message', {
777+
data: {
778+
data: {
779+
...tableDataFixture,
780+
hasRunningExperiment: false
781+
},
782+
type: MessageToWebviewType.SET_DATA
783+
}
784+
})
785+
)
786+
787+
const firstRow = screen.getByTestId('timestamp___1.exp-e7a67')
788+
fireEvent.click(firstRow)
789+
790+
const secondRow = screen.getByTestId('timestamp___1.test-branch')
791+
fireEvent.click(secondRow)
792+
793+
const target = await screen.findByText('4fb124a')
794+
fireEvent.contextMenu(target, { bubbles: true })
795+
796+
const menuitems = await screen.findAllByRole('menuitem')
797+
const itemLabels = menuitems.map(item => item.textContent)
798+
expect(itemLabels).toContain('Remove Selected Rows')
799+
})
800+
})
801+
674802
describe('Context Menu Suppression', () => {
675803
it('Suppresses the context menu on a table with no data', () => {
676804
render(<App />)

webview/src/experiments/components/Experiments.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { MessageFromWebviewType } from 'dvc/src/webview/contract'
2121
import { Table } from './table/Table'
2222
import styles from './table/styles.module.scss'
2323
import { AddColumns, Welcome } from './GetStarted'
24+
import { RowSelectionProvider } from './table/RowSelectionContext'
2425
import buildDynamicColumns from '../util/buildDynamicColumns'
2526
import { sendMessage } from '../../shared/vscode'
2627
import { WebviewWrapper } from '../../shared/components/webviewWrapper/WebviewWrapper'
@@ -223,7 +224,9 @@ export const ExperimentsTable: React.FC<{
223224

224225
return (
225226
<DragDropProvider>
226-
<Table instance={instance} tableData={tableData} />
227+
<RowSelectionProvider>
228+
<Table instance={instance} tableData={tableData} />
229+
</RowSelectionProvider>
227230
</DragDropProvider>
228231
)
229232
}

webview/src/experiments/components/table/Cell.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import cx from 'classnames'
33
import styles from './styles.module.scss'
44
import { CellProp, RowProp } from './interfaces'
55
import ClockIcon from '../../../shared/components/icons/Clock'
6+
import { clickAndEnterProps } from '../../../util/props'
67

78
const RowExpansionButton: React.FC<RowProp> = ({ row }) =>
89
row.canExpand ? (
@@ -31,8 +32,10 @@ const RowExpansionButton: React.FC<RowProp> = ({ row }) =>
3132
export const FirstCell: React.FC<
3233
CellProp & {
3334
bulletColor?: string
35+
toggleExperiment: () => void
36+
toggleRowSelection: () => void
3437
}
35-
> = ({ cell, bulletColor }) => {
38+
> = ({ cell, bulletColor, toggleExperiment, toggleRowSelection }) => {
3639
const { row, isPlaceholder } = cell
3740

3841
return (
@@ -44,14 +47,24 @@ export const FirstCell: React.FC<
4447
isPlaceholder && styles.groupPlaceholder
4548
)
4649
})}
50+
{...clickAndEnterProps(toggleRowSelection)}
4751
>
4852
<div className={styles.innerCell}>
4953
<RowExpansionButton row={row} />
50-
<span className={styles.bullet} style={{ color: bulletColor }}>
54+
<span
55+
className={styles.bullet}
56+
style={{ color: bulletColor }}
57+
{...clickAndEnterProps(toggleExperiment)}
58+
>
5159
{row.original.queued && <ClockIcon />}
5260
</span>
5361
{isPlaceholder ? null : (
54-
<div className={styles.cellContents}>{cell.render('Cell')}</div>
62+
<div
63+
className={styles.cellContents}
64+
{...clickAndEnterProps(toggleExperiment)}
65+
>
66+
{cell.render('Cell')}
67+
</div>
5568
)}
5669
</div>
5770
</div>

0 commit comments

Comments
 (0)