Skip to content

Commit 03f00c1

Browse files
authored
Improve max table depth feature (#2538)
* switch phrasing from "depth" to "height" * add a description to quick pick * add missing telemetry event * stop configuration from updating if the user does not type anything into the input box
1 parent f05b98f commit 03f00c1

File tree

14 files changed

+48
-27
lines changed

14 files changed

+48
-27
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ These are the VS Code [settings] available for the Extension:
150150
| -------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- |
151151
| `dvc.dvcPath` | Path or shell command to the DVC binary. Required unless Microsoft's [Python extension] is installed and the `dvc` package found in its environment. |
152152
| `dvc.pythonPath` | Path to the desired Python interpreter to use with DVC. Required when using a virtual environment. |
153-
| `dvc.expTableHeadMaxLayers` | Maximum depth of experiment table head rows. |
153+
| `dvc.experimentsTableHeadMaxHeight` | Maximum height of experiment table head rows. |
154154
| `dvc.doNotShowWalkthroughAfterInstall` | Do not prompt to show the Get Started page after installing. Useful for pre-configured development environments |
155155
| `dvc.doNotRecommendRedHatExtension` | Do not prompt to install the Red Hat YAML extension, which helps with DVC YAML schema validation (`dvc.yaml` and `.dvc` files). |
156156
| `dvc.doNotShowCliUnavailable` | Do not warn when the workspace contains a DVC project but the DVC binary is unavailable. |

extension/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,9 +554,9 @@
554554
"type": "string",
555555
"default": null
556556
},
557-
"dvc.expTableHeadMaxLayers": {
558-
"title": "%config.expTableHeadMaxLayers.title%",
559-
"description": "%config.expTableHeadMaxLayers.description%",
557+
"dvc.experimentsTableHeadMaxHeight": {
558+
"title": "%config.experimentsTableHeadMaxHeight.title%",
559+
"description": "%config.experimentsTableHeadMaxHeight.description%",
560560
"type": "number",
561561
"default": 5
562562
},

extension/package.nls.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@
8484
"config.doNotShowUnableToFilter.title": "Do not warn before disabling auto-apply filters to experiment selection.",
8585
"config.dvcPath.description": "Path or shell command to the DVC binary. Required unless Microsoft's Python extension is installed and the `dvc` package found in its environment.",
8686
"config.dvcPath.title": "DVC Path (or shell command)",
87-
"config.expTableHeadMaxLayers.title": "Maximum depth of Experiment table head rows",
88-
"config.expTableHeadMaxLayers.description": "Use 0 for infinite depth.",
87+
"config.experimentsTableHeadMaxHeight.title": "Maximum height of Experiment table head rows",
88+
"config.experimentsTableHeadMaxHeight.description": "Use 0 for infinite height.",
8989
"config.pythonPath.description": "Path to the desired Python interpreter to use with DVC. Required when using a virtual environment. Overrides any other extension's settings for this extension's purposes.",
9090
"config.pythonPath.title": "Python Interpreter"
9191
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ const getValue = (operator: Operator): Thenable<string | undefined> => {
9898
isFreeTextDate(text)
9999
? null
100100
: 'please enter a valid date of the form yyyy-mm-dd',
101-
getIsoDate()
101+
{ value: getIsoDate() }
102102
)
103103
}
104104
return getInput(Title.ENTER_FILTER_VALUE)

extension/src/experiments/webview/messages.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ export class WebviewMessages {
139139
this.showPlots()
140140
])
141141

142-
case MessageFromWebviewType.SET_EXPERIMENTS_HEADER_DEPTH: {
142+
case MessageFromWebviewType.SET_EXPERIMENTS_HEADER_HEIGHT: {
143143
return this.setMaxTableHeadDepth()
144144
}
145145

@@ -168,12 +168,23 @@ export class WebviewMessages {
168168

169169
private async setMaxTableHeadDepth() {
170170
const newValue = await getValidInput(
171-
Title.SET_EXPERIMENTS_HEADER_DEPTH,
171+
Title.SET_EXPERIMENTS_HEADER_HEIGHT,
172172
val => {
173173
return Number.isNaN(Number(val)) ? 'Input needs to be a number' : ''
174-
}
174+
},
175+
{ prompt: 'Use 0 for infinite height.' }
175176
)
177+
178+
if (!newValue) {
179+
return
180+
}
181+
176182
setConfigValue(ConfigKey.EXP_TABLE_HEAD_MAX_DEPTH, Number(newValue))
183+
sendTelemetryEvent(
184+
EventName.VIEWS_EXPERIMENTS_TABLE_SET_MAX_HEADER_HEIGHT,
185+
undefined,
186+
undefined
187+
)
177188
}
178189

179190
private setColumnOrder(order: string[]) {

extension/src/telemetry/constants.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ export const EventName = Object.assign(
2828
EXTENSION_EXECUTION_DETAILS_CHANGED: 'extension.executionDetails.changed',
2929
EXTENSION_LOAD: 'extension.load',
3030

31-
SET_EXPERIMENTS_HEADER_DEPTH: 'extension.updateHeaderDepth',
32-
3331
VIEWS_EXPERIMENTS_TABLE_CLOSED: 'views.experimentsTable.closed',
3432
VIEWS_EXPERIMENTS_TABLE_COLUMNS_REORDERED:
3533
'views.experimentsTable.columnsReordered',
@@ -55,6 +53,8 @@ export const EventName = Object.assign(
5553
'views.experimentsTable.selectColumns',
5654
VIEWS_EXPERIMENTS_TABLE_SELECT_EXPERIMENTS_FOR_PLOTS:
5755
'views.experimentsTable.selectExperimentsForPlots',
56+
VIEWS_EXPERIMENTS_TABLE_SET_MAX_HEADER_HEIGHT:
57+
'views.experimentsTable.updateHeaderMaxHeight',
5858
VIEWS_EXPERIMENTS_TABLE_SORT_COLUMN:
5959
'views.experimentsTable.columnSortAdded',
6060

@@ -197,8 +197,6 @@ export interface IEventNamePropertyMapping {
197197
[EventName.EXTENSION_SHOW_COMMANDS]: undefined
198198
[EventName.EXTENSION_SHOW_OUTPUT]: undefined
199199

200-
[EventName.SET_EXPERIMENTS_HEADER_DEPTH]: undefined
201-
202200
[EventName.VIEWS_EXPERIMENTS_TREE_OPENED]: DvcRootCount
203201
[EventName.VIEWS_EXPERIMENTS_FILTER_BY_TREE_OPENED]: DvcRootCount
204202
[EventName.VIEWS_EXPERIMENTS_METRICS_AND_PARAMS_TREE_OPENED]: DvcRootCount
@@ -213,6 +211,7 @@ export interface IEventNamePropertyMapping {
213211
[EventName.VIEWS_EXPERIMENTS_TABLE_RESIZE_COLUMN]: {
214212
width: number
215213
}
214+
[EventName.VIEWS_EXPERIMENTS_TABLE_SET_MAX_HEADER_HEIGHT]: undefined
216215
[EventName.VIEWS_EXPERIMENTS_TABLE_SORT_COLUMN]: SortDefinition
217216
[EventName.VIEWS_EXPERIMENTS_TABLE_REMOVE_COLUMN_SORT]: {
218217
path: string

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -882,17 +882,18 @@ suite('Experiments Test Suite', () => {
882882
it('should be able to handle a message to update the table depth', async () => {
883883
const { experiments } = buildExperiments(disposable, expShowFixture)
884884
const inputEvent = getInputBoxEvent('0')
885-
const tableMaxDepthOption = 'dvc.expTableHeadMaxLayers'
885+
const tableMaxDepthOption = 'dvc.experimentsTableHeadMaxHeight'
886886
const tableMaxDepthChanged = configurationChangeEvent(
887887
tableMaxDepthOption,
888888
disposable
889889
)
890890

891891
const webview = await experiments.showWebview()
892892

893+
const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
893894
const mockMessageReceived = getMessageReceivedEmitter(webview)
894895
mockMessageReceived.fire({
895-
type: MessageFromWebviewType.SET_EXPERIMENTS_HEADER_DEPTH
896+
type: MessageFromWebviewType.SET_EXPERIMENTS_HEADER_HEIGHT
896897
})
897898

898899
await inputEvent
@@ -902,6 +903,15 @@ suite('Experiments Test Suite', () => {
902903
expect(
903904
await workspace.getConfiguration().get(tableMaxDepthOption)
904905
).to.equal(0)
906+
expect(mockSendTelemetryEvent).to.be.calledOnce
907+
expect(
908+
mockSendTelemetryEvent,
909+
'should send a telemetry call that tells you the max height has been updated'
910+
).to.be.calledWithExactly(
911+
EventName.VIEWS_EXPERIMENTS_TABLE_SET_MAX_HEADER_HEIGHT,
912+
undefined,
913+
undefined
914+
)
905915
}).timeout(WEBVIEW_TEST_TIMEOUT)
906916

907917
it("should be able to handle a message to toggle an experiment's star status", async () => {

extension/src/vscode/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ export enum ConfigKey {
55
DO_NOT_SHOW_CLI_UNAVAILABLE = 'dvc.doNotShowCliUnavailable',
66
DO_NOT_SHOW_WALKTHROUGH_AFTER_INSTALL = 'dvc.doNotShowWalkthroughAfterInstall',
77
DO_NOT_SHOW_UNABLE_TO_FILTER = 'dvc.doNotShowUnableToFilter',
8-
EXP_TABLE_HEAD_MAX_DEPTH = 'dvc.expTableHeadMaxLayers',
8+
EXP_TABLE_HEAD_MAX_DEPTH = 'dvc.experimentsTableHeadMaxHeight',
99
DVC_PATH = 'dvc.dvcPath',
1010
PYTHON_PATH = 'dvc.pythonPath'
1111
}

extension/src/vscode/inputBox.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ export const getInput = (title: Title, value?: string) =>
1010
export const getValidInput = (
1111
title: Title,
1212
validateInput: (text?: string) => null | string,
13-
value?: string
13+
options?: { prompt?: string; value?: string }
1414
) =>
1515
window.showInputBox({
16+
prompt: options?.prompt,
1617
title,
1718
validateInput,
18-
value
19+
value: options?.value
1920
})

extension/src/vscode/title.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export enum Title {
1818
SELECT_SORT_DIRECTION = 'Select Sort Direction',
1919
SELECT_SORTS_TO_REMOVE = 'Select Sort(s) to Remove',
2020
SETUP_WORKSPACE = 'Setup the Workspace',
21-
SET_EXPERIMENTS_HEADER_DEPTH = 'Set Maximum Experiment Table Header Depth'
21+
SET_EXPERIMENTS_HEADER_HEIGHT = 'Set Maximum Experiment Table Header Height'
2222
}
2323

2424
export const getEnterValueTitle = (path: string): Title =>

0 commit comments

Comments
 (0)