Skip to content

Commit 311bbc7

Browse files
hoonjihoonji
authored
Removes hidable context menu option for columns and combines it with removable (#6741)
## Motivation for features / changes As discussed offline, it seems much less confusing to use a single "remove" operation for removing columns vs using both "remove" and "hide". ## Technical description of changes - Removes the ColumnHeader.hidable property - sets "removable" to true for scalar table columns There should be no change in functionality as the "hidable" feature hasn't been released yet.
1 parent 5ee586c commit 311bbc7

File tree

9 files changed

+22
-90
lines changed

9 files changed

+22
-90
lines changed

‎tensorboard/webapp/metrics/store/metrics_reducers.ts

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -275,47 +275,42 @@ const {initialState, reducers: namespaceContextedReducer} =
275275
removable: false,
276276
sortable: true,
277277
movable: false,
278-
hidable: false,
279278
},
280279
{
281280
type: ColumnHeaderType.SMOOTHED,
282281
name: 'smoothed',
283282
displayName: 'Smoothed',
284283
enabled: true,
285-
removable: false,
284+
removable: true,
286285
sortable: true,
287286
movable: true,
288-
hidable: true,
289287
},
290288
{
291289
type: ColumnHeaderType.VALUE,
292290
name: 'value',
293291
displayName: 'Value',
294292
enabled: true,
295-
removable: false,
293+
removable: true,
296294
sortable: true,
297295
movable: true,
298-
hidable: true,
299296
},
300297
{
301298
type: ColumnHeaderType.STEP,
302299
name: 'step',
303300
displayName: 'Step',
304301
enabled: true,
305-
removable: false,
302+
removable: true,
306303
sortable: true,
307304
movable: true,
308-
hidable: true,
309305
},
310306
{
311307
type: ColumnHeaderType.RELATIVE_TIME,
312308
name: 'relative',
313309
displayName: 'Relative',
314310
enabled: true,
315-
removable: false,
311+
removable: true,
316312
sortable: true,
317313
movable: true,
318-
hidable: true,
319314
},
320315
],
321316
rangeSelectionHeaders: [
@@ -327,127 +322,114 @@ const {initialState, reducers: namespaceContextedReducer} =
327322
removable: false,
328323
sortable: true,
329324
movable: true,
330-
hidable: true,
331325
},
332326
{
333327
type: ColumnHeaderType.MIN_VALUE,
334328
name: 'min',
335329
displayName: 'Min',
336330
enabled: true,
337-
removable: false,
331+
removable: true,
338332
sortable: true,
339333
movable: true,
340-
hidable: true,
341334
},
342335
{
343336
type: ColumnHeaderType.MAX_VALUE,
344337
name: 'max',
345338
displayName: 'Max',
346339
enabled: true,
347-
removable: false,
340+
removable: true,
348341
sortable: true,
349342
movable: true,
350-
hidable: true,
351343
},
352344
{
353345
type: ColumnHeaderType.START_VALUE,
354346
name: 'start',
355347
displayName: 'Start Value',
356348
enabled: true,
357-
removable: false,
349+
removable: true,
358350
sortable: true,
359351
movable: true,
360-
hidable: true,
361352
},
362353
{
363354
type: ColumnHeaderType.END_VALUE,
364355
name: 'end',
365356
displayName: 'End Value',
366357
enabled: true,
367-
removable: false,
358+
removable: true,
368359
sortable: true,
369360
movable: true,
370-
hidable: true,
371361
},
372362
{
373363
type: ColumnHeaderType.VALUE_CHANGE,
374364
name: 'valueChange',
375365
displayName: 'Value',
376366
enabled: true,
377-
removable: false,
367+
removable: true,
378368
sortable: true,
379369
movable: true,
380-
hidable: true,
381370
},
382371
{
383372
type: ColumnHeaderType.PERCENTAGE_CHANGE,
384373
name: 'percentageChange',
385374
displayName: '%',
386375
enabled: true,
387-
removable: false,
376+
removable: true,
388377
sortable: true,
389378
movable: true,
390-
hidable: true,
391379
},
392380
{
393381
type: ColumnHeaderType.START_STEP,
394382
name: 'startStep',
395383
displayName: 'Start Step',
396384
enabled: true,
397-
removable: false,
385+
removable: true,
398386
sortable: true,
399387
movable: true,
400-
hidable: true,
401388
},
402389
{
403390
type: ColumnHeaderType.END_STEP,
404391
name: 'endStep',
405392
displayName: 'End Step',
406393
enabled: true,
407-
removable: false,
394+
removable: true,
408395
sortable: true,
409396
movable: true,
410-
hidable: true,
411397
},
412398
{
413399
type: ColumnHeaderType.STEP_AT_MAX,
414400
name: 'stepAtMax',
415401
displayName: 'Step At Max',
416402
enabled: false,
417-
removable: false,
403+
removable: true,
418404
sortable: true,
419405
movable: true,
420-
hidable: true,
421406
},
422407
{
423408
type: ColumnHeaderType.STEP_AT_MIN,
424409
name: 'stepAtMin',
425410
displayName: 'Step At Min',
426411
enabled: false,
427-
removable: false,
412+
removable: true,
428413
sortable: true,
429414
movable: true,
430-
hidable: true,
431415
},
432416
{
433417
type: ColumnHeaderType.MEAN,
434418
name: 'mean',
435419
displayName: 'Mean',
436420
enabled: false,
437-
removable: false,
421+
removable: true,
438422
sortable: true,
439423
movable: true,
440-
hidable: true,
441424
},
442425
{
443426
type: ColumnHeaderType.RAW_CHANGE,
444427
name: 'rawChange',
445428
displayName: 'Raw',
446429
enabled: false,
447-
removable: false,
430+
removable: true,
448431
sortable: true,
449432
movable: true,
450-
hidable: true,
451433
},
452434
],
453435
filteredPluginTypes: new Set(),

‎tensorboard/webapp/metrics/views/main_view/common_selectors.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,6 @@ export const getPotentialHparamColumns = createSelector(
288288
sortable: true,
289289
movable: true,
290290
filterable: true,
291-
hidable: true,
292291
}));
293292
}
294293
);

‎tensorboard/webapp/metrics/views/main_view/common_selectors_test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ describe('common selectors', () => {
172172
removable: false,
173173
movable: false,
174174
filterable: false,
175-
hidable: false,
176175
},
177176
{
178177
type: ColumnHeaderType.CUSTOM,
@@ -1021,7 +1020,6 @@ describe('common selectors', () => {
10211020
sortable: true,
10221021
movable: true,
10231022
filterable: true,
1024-
hidable: true,
10251023
};
10261024

10271025
it('returns empty list when there are no experiments', () => {

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,13 @@ const NOTIFICATION_LAST_READ_TIME_KEY = 'notificationLastReadTimestamp';
2929
function updateScalarContextMenuOptions(headers: ColumnHeader[]) {
3030
headers.forEach((header) => {
3131
header.sortable = true;
32-
header.removable = false;
3332

3433
if (header.type === 'RUN') {
3534
header.movable = false;
36-
header.hidable = false;
35+
header.removable = false;
3736
} else {
3837
header.movable = true;
39-
header.hidable = true;
38+
header.removable = true;
4039
}
4140
});
4241
}

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,15 @@ describe('persistent_settings data_source test', () => {
215215
removable: false,
216216
sortable: true,
217217
movable: false,
218-
hidable: false,
219218
},
220219
{
221220
type: ColumnHeaderType.VALUE,
222221
name: 'value',
223222
displayName: 'Value',
224223
enabled: false,
225-
removable: false,
224+
removable: true,
226225
sortable: true,
227226
movable: true,
228-
hidable: true,
229227
},
230228
],
231229
});
@@ -263,17 +261,15 @@ describe('persistent_settings data_source test', () => {
263261
removable: false,
264262
sortable: true,
265263
movable: false,
266-
hidable: false,
267264
},
268265
{
269266
type: ColumnHeaderType.MIN_VALUE,
270267
name: 'minValue',
271268
displayName: 'Min',
272269
enabled: false,
273-
removable: false,
270+
removable: true,
274271
sortable: true,
275272
movable: true,
276-
hidable: true,
277273
},
278274
],
279275
});

‎tensorboard/webapp/runs/store/runs_reducers.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@ const {initialState: uiInitialState, reducers: uiNamespaceContextedReducers} =
324324
removable: false,
325325
movable: false,
326326
filterable: false,
327-
hidable: false,
328327
},
329328
],
330329
sortingInfo: {

‎tensorboard/webapp/runs/store/runs_selectors.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -330,16 +330,6 @@ export const getRunsTableSortingInfo = createSelector(
330330
export const getGroupedRunsTableHeaders = createSelector(
331331
getRunsTableHeaders,
332332
getDashboardDisplayedHparamColumns,
333-
(runsTableHeaders, hparamColumns) => {
334-
// Override hparam options to match runs table requirements.
335-
const columns = [...runsTableHeaders, ...hparamColumns].map((column) => {
336-
const newColumn = {...column};
337-
if (column.type === 'HPARAM') {
338-
newColumn.removable = true;
339-
newColumn.hidable = false;
340-
}
341-
return newColumn;
342-
});
343-
return dataTableUtils.groupColumns(columns);
344-
}
333+
(runsTableHeaders, hparamColumns) =>
334+
dataTableUtils.groupColumns([...runsTableHeaders, ...hparamColumns])
345335
);

‎tensorboard/webapp/runs/store/runs_selectors_test.ts

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,36 +1120,6 @@ describe('runs_selectors', () => {
11201120
}),
11211121
]);
11221122
});
1123-
1124-
it('sets the hparam column context options for the runs table', () => {
1125-
expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([
1126-
jasmine.objectContaining({
1127-
type: ColumnHeaderType.RUN,
1128-
}),
1129-
jasmine.objectContaining({
1130-
type: ColumnHeaderType.CUSTOM,
1131-
}),
1132-
jasmine.objectContaining({
1133-
type: ColumnHeaderType.HPARAM,
1134-
name: 'conv_layers',
1135-
displayName: 'Conv Layers',
1136-
enabled: true,
1137-
removable: true,
1138-
hidable: false,
1139-
}),
1140-
jasmine.objectContaining({
1141-
type: ColumnHeaderType.HPARAM,
1142-
name: 'conv_kernel_size',
1143-
displayName: 'Conv Kernel Size',
1144-
enabled: true,
1145-
removable: true,
1146-
hidable: false,
1147-
}),
1148-
jasmine.objectContaining({
1149-
type: ColumnHeaderType.COLOR,
1150-
}),
1151-
]);
1152-
});
11531123
});
11541124

11551125
describe('#getRunsTableSortingInfo', () => {

‎tensorboard/webapp/widgets/data_table/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ export declare interface ColumnHeader {
8686
sortable?: boolean;
8787
movable?: boolean;
8888
filterable?: boolean;
89-
hidable?: boolean;
9089
}
9190

9291
export enum SortingOrder {

0 commit comments

Comments
 (0)