Skip to content

Commit 0b4f375

Browse files
authored
Add select column options to table header cell context menu (#4295)
1 parent 634f7e9 commit 0b4f375

File tree

7 files changed

+116
-4
lines changed

7 files changed

+116
-4
lines changed

extension/src/experiments/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ export class Experiments extends BaseRepository<TableData> {
598598
() => this.getWebview(),
599599
() => this.notifyChanged(),
600600
() => this.selectColumns(),
601+
() => this.selectFirstColumns(),
601602
(branchesSelected: string[]) => this.selectBranches(branchesSelected),
602603
() => this.data.update()
603604
)

extension/src/experiments/webview/messages.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export class WebviewMessages {
3333
private readonly getWebview: () => BaseWebview<TableData> | undefined
3434
private readonly notifyChanged: () => void
3535
private readonly selectColumns: () => Promise<void>
36+
private readonly selectFirstColumns: () => Promise<void>
3637

3738
private readonly selectBranches: (
3839
branchesSelected: string[]
@@ -48,6 +49,7 @@ export class WebviewMessages {
4849
getWebview: () => BaseWebview<TableData> | undefined,
4950
notifyChanged: () => void,
5051
selectColumns: () => Promise<void>,
52+
selectFirstColumns: () => Promise<void>,
5153
selectBranches: (
5254
branchesSelected: string[]
5355
) => Promise<string[] | undefined>,
@@ -60,6 +62,7 @@ export class WebviewMessages {
6062
this.getWebview = getWebview
6163
this.notifyChanged = notifyChanged
6264
this.selectColumns = selectColumns
65+
this.selectFirstColumns = selectFirstColumns
6366
this.selectBranches = selectBranches
6467
this.update = update
6568
}
@@ -131,6 +134,8 @@ export class WebviewMessages {
131134

132135
case MessageFromWebviewType.SELECT_COLUMNS:
133136
return this.setColumnsStatus()
137+
case MessageFromWebviewType.SELECT_FIRST_COLUMNS:
138+
return this.setFirstColumns()
134139

135140
case MessageFromWebviewType.FOCUS_FILTERS_TREE:
136141
return this.focusFiltersTree()
@@ -322,6 +327,15 @@ export class WebviewMessages {
322327
)
323328
}
324329

330+
private setFirstColumns() {
331+
void this.selectFirstColumns()
332+
sendTelemetryEvent(
333+
EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_COLUMNS,
334+
undefined,
335+
undefined
336+
)
337+
}
338+
325339
private addColumnSort(sort: SortDefinition) {
326340
this.experiments.addSort(sort)
327341
sendTelemetryEvent(

extension/src/telemetry/constants.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ export const EventName = Object.assign(
5757
'views.experimentsTable.selectColumns',
5858
VIEWS_EXPERIMENTS_TABLE_SELECT_EXPERIMENTS_FOR_PLOTS:
5959
'views.experimentsTable.selectExperimentsForPlots',
60+
VIEWS_EXPERIMENTS_TABLE_SELECT_FIRST_COLUMNS:
61+
'views.experimentsTable.selectFirstColumns',
62+
6063
VIEWS_EXPERIMENTS_TABLE_SET_MAX_HEADER_HEIGHT:
6164
'views.experimentsTable.updateHeaderMaxHeight',
6265
VIEWS_EXPERIMENTS_TABLE_SHOW_LESS_COMMITS:
@@ -251,6 +254,7 @@ export interface IEventNamePropertyMapping {
251254
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_EXPERIMENTS_FOR_PLOTS]: {
252255
experimentCount: number
253256
}
257+
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_FIRST_COLUMNS]: undefined
254258
[EventName.VIEWS_EXPERIMENTS_TABLE_SHOW_MORE_COMMITS]: undefined
255259
[EventName.VIEWS_EXPERIMENTS_TABLE_SHOW_LESS_COMMITS]: undefined
256260
[EventName.VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE]: {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,38 @@ suite('Experiments Test Suite', () => {
10181018
expect(messageSpy).to.be.calledWithMatch(allColumnsUnselected)
10191019
}).timeout(WEBVIEW_TEST_TIMEOUT)
10201020

1021+
it('should be able to handle a message to select the first columns', async () => {
1022+
const { experiments, messageSpy } = setupExperimentsAndMockCommands()
1023+
1024+
const webview = await experiments.showWebview()
1025+
messageSpy.resetHistory()
1026+
const mockMessageReceived = getMessageReceivedEmitter(webview)
1027+
1028+
const movedColumn = 'metrics:summary.json:val_accuracy'
1029+
1030+
const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub<
1031+
[items: readonly QuickPickItem[], options: QuickPickOptionsWithTitle],
1032+
Thenable<QuickPickItemWithValue<{ path: string }>[] | undefined>
1033+
>
1034+
mockShowQuickPick.resolves([
1035+
{
1036+
value: { path: movedColumn },
1037+
label: movedColumn
1038+
}
1039+
])
1040+
1041+
const tableChangePromise = experimentsUpdatedEvent(experiments)
1042+
mockMessageReceived.fire({
1043+
type: MessageFromWebviewType.SELECT_FIRST_COLUMNS
1044+
})
1045+
await tableChangePromise
1046+
1047+
const [id, firstColumn] = messageSpy.lastCall.args[0].columnOrder
1048+
1049+
expect(id).to.equal('id')
1050+
expect(firstColumn).to.equal(movedColumn)
1051+
}).timeout(WEBVIEW_TEST_TIMEOUT)
1052+
10211053
it('should be able to handle a message to focus the sorts tree', async () => {
10221054
const { experiments } = buildExperiments({ disposer: disposable })
10231055

extension/src/webview/contract.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export enum MessageFromWebviewType {
5151
HIDE_EXPERIMENTS_TABLE_COLUMN = 'hide-experiments-table-column',
5252
SELECT_EXPERIMENTS = 'select-experiments',
5353
SELECT_COLUMNS = 'select-columns',
54+
SELECT_FIRST_COLUMNS = 'select-first-columns',
5455
SELECT_PLOTS = 'select-plots',
5556
SET_EXPERIMENTS_FOR_PLOTS = 'set-experiments-for-plots',
5657
SET_EXPERIMENTS_AND_OPEN_PLOTS = 'set-experiments-and-open-plots',
@@ -216,6 +217,7 @@ export type MessageFromWebview =
216217
| { type: MessageFromWebviewType.REFRESH_EXP_DATA }
217218
| { type: MessageFromWebviewType.REFRESH_REVISIONS }
218219
| { type: MessageFromWebviewType.SELECT_COLUMNS }
220+
| { type: MessageFromWebviewType.SELECT_FIRST_COLUMNS }
219221
| { type: MessageFromWebviewType.FOCUS_FILTERS_TREE }
220222
| { type: MessageFromWebviewType.FOCUS_SORTS_TREE }
221223
| { type: MessageFromWebviewType.OPEN_PLOTS_WEBVIEW }

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

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -737,10 +737,10 @@ describe('App', () => {
737737
advanceTimersByTime(100)
738738

739739
const menuitems = screen.getAllByRole('menuitem')
740-
expect(menuitems).toHaveLength(6)
740+
expect(menuitems).toHaveLength(8)
741741
expect(
742742
menuitems.filter(item => !item.className.includes('disabled'))
743-
).toHaveLength(3)
743+
).toHaveLength(5)
744744

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

763763
disabledMenuItem && fireEvent.click(disabledMenuItem, { bubbles: true })
764-
expect(screen.queryAllByRole('menuitem')).toHaveLength(6)
764+
expect(screen.queryAllByRole('menuitem')).toHaveLength(8)
765765
})
766766

767767
it('should have the same enabled options in the empty placeholders', () => {
@@ -783,6 +783,8 @@ describe('App', () => {
783783
expect(menuitems).toStrictEqual([
784784
'Hide Column',
785785
'Set Max Header Height',
786+
'Select Columns',
787+
'Select First Columns',
786788
'Sort Ascending',
787789
'Sort Descending'
788790
])
@@ -807,12 +809,54 @@ describe('App', () => {
807809
.filter(item => !item.className.includes('disabled'))
808810
.map(item => item.textContent)
809811

810-
expect(menuitems).toStrictEqual(['Set Max Header Height'])
812+
expect(menuitems).toStrictEqual([
813+
'Set Max Header Height',
814+
'Select Columns',
815+
'Select First Columns'
816+
])
811817

812818
fireEvent.keyDown(segment, { bubbles: true, key: 'Escape' })
813819
}
814820
})
815821

822+
it('should send the correct message when Select Columns is clicked', () => {
823+
renderTableWithPlaceholder()
824+
const placeholders = screen.getAllByTestId(/header-Created/)
825+
const placeholder = placeholders[0]
826+
fireEvent.contextMenu(placeholder, { bubbles: true })
827+
advanceTimersByTime(100)
828+
829+
const selectOption = screen.getByText('Select Columns')
830+
831+
mockPostMessage.mockClear()
832+
833+
fireEvent.click(selectOption)
834+
835+
expect(mockPostMessage).toHaveBeenCalledTimes(1)
836+
expect(mockPostMessage).toHaveBeenCalledWith({
837+
type: MessageFromWebviewType.SELECT_COLUMNS
838+
})
839+
})
840+
841+
it('should send the correct message when Select First Columns is clicked', () => {
842+
renderTableWithPlaceholder()
843+
const placeholders = screen.getAllByTestId(/header-Created/)
844+
const placeholder = placeholders[0]
845+
fireEvent.contextMenu(placeholder, { bubbles: true })
846+
advanceTimersByTime(100)
847+
848+
const selectOption = screen.getByText('Select First Columns')
849+
850+
mockPostMessage.mockClear()
851+
852+
fireEvent.click(selectOption)
853+
854+
expect(mockPostMessage).toHaveBeenCalledTimes(1)
855+
expect(mockPostMessage).toHaveBeenCalledWith({
856+
type: MessageFromWebviewType.SELECT_FIRST_COLUMNS
857+
})
858+
})
859+
816860
describe('Hiding a column from its empty placeholder', () => {
817861
it('should send the column id and not the placeholder id as the message payload', () => {
818862
renderTableWithPlaceholder()

webview/src/experiments/components/table/header/ContextMenuContent.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,26 @@ export const getMenuOptions = (
114114
}
115115
},
116116
{
117+
divider: true,
117118
id: 'update-header-depth',
118119
label: 'Set Max Header Height',
119120
message: {
120121
type: MessageFromWebviewType.SET_EXPERIMENTS_HEADER_HEIGHT
121122
}
123+
},
124+
{
125+
id: 'select-columns',
126+
label: 'Select Columns',
127+
message: {
128+
type: MessageFromWebviewType.SELECT_COLUMNS
129+
}
130+
},
131+
{
132+
id: 'select-first-columns',
133+
label: 'Select First Columns',
134+
message: {
135+
type: MessageFromWebviewType.SELECT_FIRST_COLUMNS
136+
}
122137
}
123138
]
124139

0 commit comments

Comments
 (0)