Skip to content

Commit

Permalink
Experiment table open to the side (#1796)
Browse files Browse the repository at this point in the history
* Add Open-to-the-side option to table header context-menu

* Remove commented code

* Add test for the Open-to-the-side message handler

* Add a telemetry event for the open file message

* Refactor array utils

* Clean the file segment of column paths before using them

* Limit Hide column option to leaves only
  • Loading branch information
wolmir authored Jun 2, 2022
1 parent d6ea272 commit 1d572b5
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 35 deletions.
5 changes: 3 additions & 2 deletions extension/src/experiments/columns/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ export const splitColumnPath = (path: string) => {
if (!fileSegment) {
return [baseSegment]
}
const cleanFileSegment = fileSegment.replace(/_\d+$/g, '')
if (!paramPath) {
return [baseSegment, fileSegment]
return [baseSegment, cleanFileSegment]
}
return [
baseSegment,
fileSegment,
cleanFileSegment,
...paramPath.split(METRIC_PARAM_SEPARATOR).map(decodeColumn)
]
}
18 changes: 17 additions & 1 deletion extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event, EventEmitter, Memento, window } from 'vscode'
import { join } from 'path'
import { Event, EventEmitter, Memento, Uri, ViewColumn, window } from 'vscode'
import { ExperimentsModel } from './model'
import { pickExperiments } from './model/quickPicks'
import { pickAndModifyParams } from './model/queue/quickPick'
Expand All @@ -15,6 +16,7 @@ import { ExperimentsData } from './data'
import { askToDisableAutoApplyFilters } from './toast'
import { Experiment, ColumnType, TableData } from './webview/contract'
import { SortDefinition } from './model/sortBy'
import { splitColumnPath } from './columns/paths'
import { ResourceLocator } from '../resourceLocator'
import {
AvailableCommands,
Expand Down Expand Up @@ -380,6 +382,18 @@ export class Experiments extends BaseRepository<TableData> {
)
}

private async openParamsFileToTheSide(path: string) {
const [, fileSegment] = splitColumnPath(path)
await window.showTextDocument(Uri.file(join(this.dvcRoot, fileSegment)), {
viewColumn: ViewColumn.Beside
})
sendTelemetryEvent(
EventName.VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE,
{ path },
undefined
)
}

private setupInitialData() {
const waitForInitialData = this.dispose.track(
this.onDidChangeExperiments(() => {
Expand Down Expand Up @@ -440,6 +454,8 @@ export class Experiments extends BaseRepository<TableData> {
return this.setExperimentStatus(message.payload)
case MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN:
return this.hideTableColumn(message.payload)
case MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE:
return this.openParamsFileToTheSide(message.payload)
case MessageFromWebviewType.SORT_COLUMN:
return this.addColumnSort(message.payload)
case MessageFromWebviewType.REMOVE_COLUMN_SORT:
Expand Down
5 changes: 5 additions & 0 deletions extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export const EventName = Object.assign(
VIEWS_EXPERIMENTS_TABLE_FOCUS_CHANGED:
'views.experimentsTable.focusChanged',
VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN: 'views.experimentsTable.columnHidden',
VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE:
'views.experimentsTable.paramsFileOpened',
VIEWS_EXPERIMENTS_TABLE_REMOVE_COLUMN_SORT:
'views.experimentsTable.columnSortRemoved',
VIEWS_EXPERIMENTS_TABLE_RESIZE_COLUMN:
Expand Down Expand Up @@ -196,6 +198,9 @@ export interface IEventNamePropertyMapping {
path: string
}
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_COLUMNS]: undefined
[EventName.VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE]: {
path: string
}

[EventName.VIEWS_PLOTS_CLOSED]: undefined
[EventName.VIEWS_PLOTS_CREATED]: undefined
Expand Down
47 changes: 46 additions & 1 deletion extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
commands,
workspace,
Uri,
QuickPickItem
QuickPickItem,
ViewColumn
} from 'vscode'
import { buildExperiments } from './util'
import { Disposable } from '../../../extension'
Expand Down Expand Up @@ -390,6 +391,50 @@ suite('Experiments Test Suite', () => {
)
})

it('should be able to handle a message to open the source params file from a column path', async () => {
const { experiments } = setupExperimentsAndMockCommands()

const mockShowTextDocument = stub(window, 'showTextDocument')
const webview = await experiments.showWebview()
const mockMessageReceived = getMessageReceivedEmitter(webview)
const mockColumnId = 'params:params.yaml_5'

mockMessageReceived.fire({
payload: mockColumnId,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
})

expect(mockShowTextDocument).to.be.calledOnce
expect(mockShowTextDocument).to.be.calledWithExactly(
Uri.file(join(dvcDemoPath, 'params.yaml')),
{
viewColumn: ViewColumn.Beside
}
)
})

it('should be able to handle a message to open different params files than the default one', async () => {
const { experiments } = setupExperimentsAndMockCommands()

const mockShowTextDocument = stub(window, 'showTextDocument')
const webview = await experiments.showWebview()
const mockMessageReceived = getMessageReceivedEmitter(webview)
const mockColumnId = 'params:params_alt.json_5:nested1.nested2'

mockMessageReceived.fire({
payload: mockColumnId,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
})

expect(mockShowTextDocument).to.be.calledOnce
expect(mockShowTextDocument).to.be.calledWithExactly(
Uri.file(join(dvcDemoPath, 'params_alt.json')),
{
viewColumn: ViewColumn.Beside
}
)
})

it('should be able to handle a message to apply an experiment to workspace', async () => {
const { experiments, mockExecuteCommand } =
setupExperimentsAndMockCommands()
Expand Down
5 changes: 5 additions & 0 deletions extension/src/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export enum MessageFromWebviewType {
INITIALIZED = 'initialized',
APPLY_EXPERIMENT_TO_WORKSPACE = 'apply-experiment-to-workspace',
CREATE_BRANCH_FROM_EXPERIMENT = 'create-branch-from-experiment',
OPEN_PARAMS_FILE_TO_THE_SIDE = 'open-params-file-to-the-side',
REMOVE_COLUMN_SORT = 'remove-column-sort',
REMOVE_EXPERIMENT = 'remove-experiment',
RENAME_SECTION = 'rename-section',
Expand Down Expand Up @@ -68,6 +69,10 @@ export type MessageFromWebview =
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
payload: string
}
| {
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
payload: string
}
| {
type: MessageFromWebviewType.APPLY_EXPERIMENT_TO_WORKSPACE
payload: string
Expand Down
14 changes: 6 additions & 8 deletions webview/src/experiments/components/table/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { sendMessage } from '../../../shared/vscode'
import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu'
import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu'
import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption'
import { pushIf } from '../../../util/array'

const getExperimentTypeClass = ({ running, queued, selected }: Experiment) => {
if (running) {
Expand Down Expand Up @@ -52,10 +53,7 @@ export const RowContextMenu: React.FC<RowProp> = ({
const contextMenuOptions = React.useMemo(() => {
const menuOptions: MessagesMenuOptionProps[] = []

const pushIf = (condition: boolean, options: MessagesMenuOptionProps[]) =>
condition && menuOptions.push(...options)

pushIf(!queued && !isWorkspace && depth > 0, [
pushIf(menuOptions, !queued && !isWorkspace && depth > 0, [
experimentMenuOption(
id,
'Apply to Workspace',
Expand All @@ -68,7 +66,7 @@ export const RowContextMenu: React.FC<RowProp> = ({
)
])

pushIf(depth === 1, [
pushIf(menuOptions, depth === 1, [
experimentMenuOption(
id,
'Remove',
Expand All @@ -78,23 +76,23 @@ export const RowContextMenu: React.FC<RowProp> = ({

const isNotCheckpoint = depth <= 1 || isWorkspace

pushIf(isNotCheckpoint, [
pushIf(menuOptions, isNotCheckpoint, [
experimentMenuOption(
id,
projectHasCheckpoints ? 'Modify and Resume' : 'Modify and Run',
MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_AND_RUN
)
])

pushIf(isNotCheckpoint && projectHasCheckpoints, [
pushIf(menuOptions, isNotCheckpoint && projectHasCheckpoints, [
experimentMenuOption(
id,
'Modify, Reset and Run',
MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_RESET_AND_RUN
)
])

pushIf(isNotCheckpoint, [
pushIf(menuOptions, isNotCheckpoint, [
experimentMenuOption(
id,
'Modify and Queue',
Expand Down
59 changes: 39 additions & 20 deletions webview/src/experiments/components/table/TableHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
OnDrop
} from '../../../shared/components/dragDrop/DragDropWorkbench'
import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu'
import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption'

export const ColumnDragHandle: React.FC<{
disabled: boolean
Expand Down Expand Up @@ -197,6 +198,31 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
})
}

const contextMenuOptions: MessagesMenuOptionProps[] = React.useMemo(() => {
const menuOptions: MessagesMenuOptionProps[] = [
{
hidden: !!column.headers,
id: 'hide-column',
label: 'Hide Column',
message: {
payload: column.id,
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
}
},
{
hidden: column.group !== ColumnType.PARAMS,
id: 'open-to-the-side',
label: 'Open to the Side',
message: {
payload: column.id,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
}
}
]

return menuOptions
}, [column])

return (
<TableHeaderCell
column={column}
Expand All @@ -206,28 +232,21 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
onDragOver={onDragOver}
onDragStart={onDragStart}
onDrop={onDrop}
menuDisabled={!isSortable}
menuDisabled={!isSortable && column.group !== ColumnType.PARAMS}
menuContent={
<div>
<SortPicker
sortOrder={sortOrder}
setSelectedOrder={order => {
setColumnSort(order)
}}
/>
<VSCodeDivider />
<MessagesMenu
options={[
{
id: 'hide-column',
label: 'Hide Column',
message: {
payload: column.id,
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
}
}
]}
/>
{isSortable && (
<div>
<SortPicker
sortOrder={sortOrder}
setSelectedOrder={order => {
setColumnSort(order)
}}
/>
<VSCodeDivider />
</div>
)}
<MessagesMenu options={contextMenuOptions} />
</div>
}
/>
Expand Down
8 changes: 5 additions & 3 deletions webview/src/shared/components/messagesMenu/MessagesMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ export interface MessagesMenuProps {

export const MessagesMenu: React.FC<MessagesMenuProps> = ({ options }) => (
<div role="menu">
{options.map((option, i) => (
<MessagesMenuOption key={option.id} {...option} index={i} />
))}
{options
.filter(({ hidden }) => !hidden)
.map((option, i) => (
<MessagesMenuOption key={option.id} {...option} index={i} />
))}
</div>
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface MessagesMenuOptionProps {
id: string
label: string
message: MessageFromWebview
hidden?: boolean
}

interface MessagesMenuOptionAllProps extends MessagesMenuOptionProps {
Expand Down
2 changes: 2 additions & 0 deletions webview/src/util/array.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const pushIf = <T>(array: T[], condition: boolean, elements: T[]) =>
condition && array.push(...elements)

0 comments on commit 1d572b5

Please sign in to comment.