Skip to content

Commit

Permalink
Add move to start option to experiment table header cell context menu
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Jul 19, 2023
1 parent a092aca commit dde25e0
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 3 deletions.
23 changes: 23 additions & 0 deletions extension/src/experiments/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ export class WebviewMessages {
return this.setExperimentStars(message.payload)
case MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN:
return this.hideTableColumn(message.payload)
case MessageFromWebviewType.EXPERIMENTS_TABLE_MOVE_TO_START:
return this.movePathToStart(message.payload)
case MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE:
return this.openParamsFileToTheSide(message.payload)
case MessageFromWebviewType.SORT_COLUMN:
Expand Down Expand Up @@ -392,6 +394,27 @@ export class WebviewMessages {
)
}

private movePathToStart(path: string) {
const toMove = []
const terminalNodes = this.columns.getColumnOrder()
for (const terminalNode of terminalNodes) {
if (!terminalNode.startsWith(path)) {
continue
}
toMove.push(terminalNode)
}

this.columns.selectFirst(toMove)

this.notifyChanged()

sendTelemetryEvent(
EventName.VIEWS_EXPERIMENTS_TABLE_MOVE_TO_START,
{ path },
undefined
)
}

private async openParamsFileToTheSide(path: string) {
const [, fileSegment] = splitColumnPath(path)
await window.showTextDocument(Uri.file(join(this.dvcRoot, fileSegment)), {
Expand Down
2 changes: 2 additions & 0 deletions extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const EventName = Object.assign(
VIEWS_EXPERIMENTS_TABLE_FOCUS_SORTS_TREE:
'views.experimentsTable.focusSortsTree',
VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN: 'views.experimentsTable.columnHidden',
VIEWS_EXPERIMENTS_TABLE_MOVE_TO_START: 'views.experimentsTable.moveToStart',
VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE:
'views.experimentsTable.paramsFileOpened',
VIEWS_EXPERIMENTS_TABLE_REFRESH: 'views.experimentsTable.refresh',
Expand Down Expand Up @@ -249,6 +250,7 @@ export interface IEventNamePropertyMapping {
[EventName.VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN]: {
path: string
}
[EventName.VIEWS_EXPERIMENTS_TABLE_MOVE_TO_START]: { path: string }
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_BRANCHES]: undefined
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_COLUMNS]: undefined
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_EXPERIMENTS_FOR_PLOTS]: {
Expand Down
44 changes: 44 additions & 0 deletions extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ suite('Experiments Test Suite', () => {
}).timeout(WEBVIEW_TEST_TIMEOUT)
})

// eslint-disable-next-line sonarjs/cognitive-complexity
describe('handleMessageFromWebview', () => {
after(() =>
workspace
Expand Down Expand Up @@ -1050,6 +1051,49 @@ suite('Experiments Test Suite', () => {
expect(firstColumn).to.equal(movedColumn)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should be able to handle a message to move a column group to the start of the table', async () => {
const { experiments, messageSpy } = setupExperimentsAndMockCommands()

const webview = await experiments.showWebview()
messageSpy.resetHistory()
const mockMessageReceived = getMessageReceivedEmitter(webview)

const movedGroup = 'params:params.yaml'

const tableChangePromise = experimentsUpdatedEvent(experiments)
mockMessageReceived.fire({
payload: movedGroup,
type: MessageFromWebviewType.EXPERIMENTS_TABLE_MOVE_TO_START
})
await tableChangePromise

const paramsYamlColumns = columnsOrderFixture.filter(column =>
column.startsWith('params:params.yaml')
).length

expect(paramsYamlColumns).to.be.greaterThan(6)

const [id, ...columns] = messageSpy.lastCall.args[0].columnOrder
expect(id).to.equal('id')

let params = 0
let other = 0
for (const column of columns) {
if (column.startsWith('params:params.yaml')) {
params = params + 1
} else {
other = other + 1
}
if (params < paramsYamlColumns) {
expect(
other,
'all params:params.yaml entries are at the start of the order'
).to.equal(0)
}
}
expect(params).to.equal(paramsYamlColumns)
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should be able to handle a message to focus the sorts tree', async () => {
const { experiments } = buildExperiments({ disposer: disposable })

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 @@ -49,6 +49,7 @@ export enum MessageFromWebviewType {
TOGGLE_EXPERIMENT = 'toggle-experiment',
TOGGLE_EXPERIMENT_STAR = 'toggle-experiment-star',
HIDE_EXPERIMENTS_TABLE_COLUMN = 'hide-experiments-table-column',
EXPERIMENTS_TABLE_MOVE_TO_START = 'experiments-table-move-to-start',
SELECT_EXPERIMENTS = 'select-experiments',
SELECT_COLUMNS = 'select-columns',
SELECT_FIRST_COLUMNS = 'select-first-columns',
Expand Down Expand Up @@ -134,6 +135,10 @@ export type MessageFromWebview =
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
payload: string
}
| {
type: MessageFromWebviewType.EXPERIMENTS_TABLE_MOVE_TO_START
payload: string
}
| {
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
payload: string
Expand Down
27 changes: 24 additions & 3 deletions webview/src/experiments/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -737,10 +737,10 @@ describe('App', () => {
advanceTimersByTime(100)

const menuitems = screen.getAllByRole('menuitem')
expect(menuitems).toHaveLength(8)
expect(menuitems).toHaveLength(9)
expect(
menuitems.filter(item => !item.className.includes('disabled'))
).toHaveLength(5)
).toHaveLength(6)

fireEvent.keyDown(paramsFileHeader, { bubbles: true, key: 'Escape' })
expect(screen.queryAllByRole('menuitem')).toHaveLength(0)
Expand All @@ -761,7 +761,7 @@ describe('App', () => {
expect(disabledMenuItem).toBeDefined()

disabledMenuItem && fireEvent.click(disabledMenuItem, { bubbles: true })
expect(screen.queryAllByRole('menuitem')).toHaveLength(8)
expect(screen.queryAllByRole('menuitem')).toHaveLength(9)
})

it('should have the same enabled options in the empty placeholders', () => {
Expand All @@ -782,6 +782,7 @@ describe('App', () => {

expect(menuitems).toStrictEqual([
'Hide Column',
'Move to Start',
'Set Max Header Height',
'Select Columns',
'Select First Columns',
Expand Down Expand Up @@ -819,6 +820,26 @@ describe('App', () => {
}
})

it('should send the correct message when Move to Start is clicked', () => {
renderTableWithPlaceholder()
const placeholders = screen.getAllByTestId(/header-Created/)
const placeholder = placeholders[0]
fireEvent.contextMenu(placeholder, { bubbles: true })
advanceTimersByTime(100)

const moveOption = screen.getByText('Move to Start')

mockPostMessage.mockClear()

fireEvent.click(moveOption)

expect(mockPostMessage).toHaveBeenCalledTimes(1)
expect(mockPostMessage).toHaveBeenCalledWith({
payload: 'Created',
type: MessageFromWebviewType.EXPERIMENTS_TABLE_MOVE_TO_START
})
})

it('should send the correct message when Select Columns is clicked', () => {
renderTableWithPlaceholder()
const placeholders = screen.getAllByTestId(/header-Created/)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ export const getMenuOptions = (
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
}
},
{
disabled: isFromExperimentColumn(header),
id: 'move-to-start',
label: 'Move to Start',
message: {
payload: leafColumn.id,
type: MessageFromWebviewType.EXPERIMENTS_TABLE_MOVE_TO_START
}
},
{
disabled:
(header.column.columnDef as ColumnWithGroup).group !==
Expand Down

0 comments on commit dde25e0

Please sign in to comment.