Skip to content

Commit a983570

Browse files
Add global sort and filter indicators to the Experiments Table webview (#1872)
* Add basic sorting indicator * Add filters and refactor into external file * Change design to icons with badges * Fix storybook and add a sort-less scenario * Add jest tests * Add tree focus click events * Tweak styles and add postMessage tests * Use SCSS variables * Use consistent icons * Fix svg fill issues with alternate icon usage * Add telemetry and integration tests * Add basic tooltips * Add filtered counts to tooltips * Add tooltip tests * Use "No" instead of 0 in filter tooltips Co-authored-by: mattseddon <[email protected]>
1 parent 4739b79 commit a983570

File tree

21 files changed

+513
-5
lines changed

21 files changed

+513
-5
lines changed

extension/src/experiments/index.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import { join } from 'path'
2-
import { Event, EventEmitter, Memento, Uri, ViewColumn, window } from 'vscode'
2+
import {
3+
commands,
4+
Event,
5+
EventEmitter,
6+
Memento,
7+
Uri,
8+
ViewColumn,
9+
window
10+
} from 'vscode'
311
import { ExperimentsModel } from './model'
412
import { pickExperiments } from './model/quickPicks'
513
import { pickAndModifyParams } from './model/modify/quickPick'
@@ -18,6 +26,7 @@ import { Experiment, ColumnType, TableData } from './webview/contract'
1826
import { DecorationProvider } from './model/filterBy/decorationProvider'
1927
import { SortDefinition } from './model/sortBy'
2028
import { splitColumnPath } from './columns/paths'
29+
import { collectFilteredCounts } from './model/filterBy/collect'
2130
import { ResourceLocator } from '../resourceLocator'
2231
import {
2332
AvailableCommands,
@@ -390,6 +399,30 @@ export class Experiments extends BaseRepository<TableData> {
390399
return this.experiments.hasRunningExperiment()
391400
}
392401

402+
private focusSortsTree() {
403+
const commandPromise = commands.executeCommand(
404+
'dvc.views.experimentsSortByTree.focus'
405+
)
406+
sendTelemetryEvent(
407+
EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_SORTS_TREE,
408+
undefined,
409+
undefined
410+
)
411+
return commandPromise
412+
}
413+
414+
private focusFiltersTree() {
415+
const commandPromise = commands.executeCommand(
416+
'dvc.views.experimentsFilterByTree.focus'
417+
)
418+
sendTelemetryEvent(
419+
EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_FILTERS_TREE,
420+
undefined,
421+
undefined
422+
)
423+
return commandPromise
424+
}
425+
393426
private hideTableColumn(path: string) {
394427
this.toggleColumnStatus(path)
395428
sendTelemetryEvent(
@@ -453,6 +486,7 @@ export class Experiments extends BaseRepository<TableData> {
453486
columnOrder: this.columns.getColumnOrder(),
454487
columnWidths: this.columns.getColumnWidths(),
455488
columns: this.columns.getSelected(),
489+
filteredCounts: collectFilteredCounts(this.getFilteredExperiments()),
456490
filters: this.experiments.getFilterPaths(),
457491
hasCheckpoints: this.hasCheckpoints(),
458492
hasColumns: this.columns.hasColumns(),
@@ -507,6 +541,12 @@ export class Experiments extends BaseRepository<TableData> {
507541
return this.removeExperiment(message.payload)
508542
case MessageFromWebviewType.SELECT_COLUMNS:
509543
return this.setColumnsStatus()
544+
545+
case MessageFromWebviewType.FOCUS_FILTERS_TREE:
546+
return this.focusFiltersTree()
547+
case MessageFromWebviewType.FOCUS_SORTS_TREE:
548+
return this.focusSortsTree()
549+
510550
default:
511551
Logger.error(`Unexpected message: ${JSON.stringify(message)}`)
512552
}

extension/src/experiments/model/filterBy/collect.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ import { definedAndNonEmpty } from '../../../util/array'
88
import { Experiment } from '../../webview/contract'
99

1010
export type ExperimentWithType = Experiment & { type: ExperimentType }
11+
export type FilteredCounts = {
12+
checkpoints: number
13+
experiments: number
14+
}
1115

1216
export const collectFilteredCounts = (
1317
experiments: { type: ExperimentType }[]
14-
) => {
18+
): FilteredCounts => {
1519
const filtered = { checkpoints: 0, experiments: 0 }
1620

1721
for (const { type } of experiments) {

extension/src/experiments/webview/contract.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { BaseExperimentFields, ValueTree } from '../../cli/reader'
2+
import { FilteredCounts } from '../model/filterBy/collect'
23
import { SortDefinition } from '../model/sortBy'
34

45
export interface MetricOrParamColumns {
@@ -60,6 +61,7 @@ export type TableData = {
6061
hasRunningExperiment: boolean
6162
rows: Row[]
6263
sorts: SortDefinition[]
64+
filteredCounts: FilteredCounts
6365
filters: string[]
6466
}
6567

extension/src/telemetry/constants.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ export const EventName = Object.assign(
3636
'views.experimentTable.toggleStatus',
3737
VIEWS_EXPERIMENTS_TABLE_FOCUS_CHANGED:
3838
'views.experimentsTable.focusChanged',
39+
VIEWS_EXPERIMENTS_TABLE_FOCUS_FILTERS_TREE:
40+
'views.experimentsTable.focusFiltersTree',
41+
VIEWS_EXPERIMENTS_TABLE_FOCUS_SORTS_TREE:
42+
'views.experimentsTable.focusSortsTree',
3943
VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN: 'views.experimentsTable.columnHidden',
4044
VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE:
4145
'views.experimentsTable.paramsFileOpened',
@@ -185,6 +189,8 @@ export interface IEventNamePropertyMapping {
185189
[EventName.VIEWS_EXPERIMENTS_TABLE_EXPERIMENT_TOGGLE]: undefined
186190
[EventName.VIEWS_EXPERIMENTS_TABLE_CLOSED]: undefined
187191
[EventName.VIEWS_EXPERIMENTS_TABLE_COLUMNS_REORDERED]: undefined
192+
[EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_FILTERS_TREE]: undefined
193+
[EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_SORTS_TREE]: undefined
188194
[EventName.VIEWS_EXPERIMENTS_TABLE_RESIZE_COLUMN]: {
189195
width: number
190196
}

extension/src/test/fixtures/expShow/deeplyNested.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ const deeplyNestedTableData: TableData = {
281281
changes: [],
282282
columnOrder: [],
283283
columnWidths: {},
284+
filteredCounts: { experiments: 0, checkpoints: 0 },
284285
filters: [
285286
'params:params.yaml:nested1.doubled',
286287
'params:params.yaml:nested1%2Enested2%2Enested3.nested4.nested5b.doubled'

extension/src/test/fixtures/expShow/tableData.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import rowsFixture from './rows'
33
import columnsFixture from './columns'
44

55
const tableDataFixture: TableData = {
6+
filteredCounts: { experiments: 0, checkpoints: 0 },
67
rows: rowsFixture,
78
columns: columnsFixture,
89
filters: [],

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ suite('Experiments Test Suite', () => {
126126
columnOrder: [],
127127
columnWidths: {},
128128
columns: columnsFixture,
129+
filteredCounts: { checkpoints: 0, experiments: 0 },
129130
filters: [],
130131
hasCheckpoints: true,
131132
hasColumns: true,
@@ -690,6 +691,7 @@ suite('Experiments Test Suite', () => {
690691
columnOrder: [],
691692
columnWidths: {},
692693
columns: [],
694+
filteredCounts: { checkpoints: 0, experiments: 0 },
693695
filters: [],
694696
hasCheckpoints: true,
695697
hasColumns: true,
@@ -700,6 +702,72 @@ suite('Experiments Test Suite', () => {
700702

701703
expect(messageSpy).to.be.calledWith(allColumnsUnselected)
702704
})
705+
706+
it('should be able to handle a message to focus the sorts tree', async () => {
707+
const { experiments } = buildExperiments(disposable, expShowFixture)
708+
709+
const webview = await experiments.showWebview()
710+
711+
const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
712+
const mockMessageReceived = getMessageReceivedEmitter(webview)
713+
const executeCommandStub = stub(commands, 'executeCommand')
714+
715+
const messageReceived = new Promise(resolve =>
716+
disposable.track(mockMessageReceived.event(() => resolve(undefined)))
717+
)
718+
719+
mockMessageReceived.fire({
720+
type: MessageFromWebviewType.FOCUS_SORTS_TREE
721+
})
722+
723+
expect(executeCommandStub).to.be.calledWith(
724+
'dvc.views.experimentsSortByTree.focus'
725+
)
726+
727+
await messageReceived
728+
expect(mockSendTelemetryEvent).to.be.calledOnce
729+
expect(
730+
mockSendTelemetryEvent,
731+
'should send a telemetry call that the sorts tree has been focused'
732+
).to.be.calledWithExactly(
733+
EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_SORTS_TREE,
734+
undefined,
735+
undefined
736+
)
737+
})
738+
739+
it('should be able to handle a message to focus the filters tree', async () => {
740+
const { experiments } = buildExperiments(disposable, expShowFixture)
741+
742+
const webview = await experiments.showWebview()
743+
744+
const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
745+
const mockMessageReceived = getMessageReceivedEmitter(webview)
746+
const executeCommandStub = stub(commands, 'executeCommand')
747+
748+
const messageReceived = new Promise(resolve =>
749+
disposable.track(mockMessageReceived.event(() => resolve(undefined)))
750+
)
751+
752+
mockMessageReceived.fire({
753+
type: MessageFromWebviewType.FOCUS_FILTERS_TREE
754+
})
755+
756+
expect(executeCommandStub).to.be.calledWith(
757+
'dvc.views.experimentsFilterByTree.focus'
758+
)
759+
760+
await messageReceived
761+
expect(mockSendTelemetryEvent).to.be.calledOnce
762+
expect(
763+
mockSendTelemetryEvent,
764+
'should send a telemetry call that the filters tree has been focused'
765+
).to.be.calledWithExactly(
766+
EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_FILTERS_TREE,
767+
undefined,
768+
undefined
769+
)
770+
})
703771
})
704772

705773
describe('Sorting', () => {

extension/src/test/suite/experiments/model/filterBy/tree.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
117117
columnOrder: [],
118118
columnWidths: {},
119119
columns: columnsFixture,
120+
filteredCounts: { checkpoints: 4, experiments: 1 },
120121
filters: [accuracyPath],
121122
hasCheckpoints: true,
122123
hasColumns: true,
@@ -147,6 +148,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
147148
columnOrder: [],
148149
columnWidths: {},
149150
columns: columnsFixture,
151+
filteredCounts: { checkpoints: 0, experiments: 0 },
150152
filters: [],
151153
hasCheckpoints: true,
152154
hasColumns: true,

extension/src/webview/contract.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ export enum MessageFromWebviewType {
1414
INITIALIZED = 'initialized',
1515
APPLY_EXPERIMENT_TO_WORKSPACE = 'apply-experiment-to-workspace',
1616
CREATE_BRANCH_FROM_EXPERIMENT = 'create-branch-from-experiment',
17+
FOCUS_FILTERS_TREE = 'focus-filters-tree',
18+
FOCUS_SORTS_TREE = 'focus-sorts-tree',
1719
OPEN_PARAMS_FILE_TO_THE_SIDE = 'open-params-file-to-the-side',
1820
REMOVE_COLUMN_SORT = 'remove-column-sort',
1921
REMOVE_EXPERIMENT = 'remove-experiment',
@@ -135,6 +137,8 @@ export type MessageFromWebview =
135137
| { type: MessageFromWebviewType.REFRESH_REVISION; payload: string }
136138
| { type: MessageFromWebviewType.REFRESH_REVISIONS; payload: string[] }
137139
| { type: MessageFromWebviewType.SELECT_COLUMNS }
140+
| { type: MessageFromWebviewType.FOCUS_FILTERS_TREE }
141+
| { type: MessageFromWebviewType.FOCUS_SORTS_TREE }
138142

139143
export type MessageToWebview<T extends WebviewData> = {
140144
type: MessageToWebviewType.SET_DATA

0 commit comments

Comments
 (0)