Skip to content

Commit 189e0f8

Browse files
hoonjihoonji
authored
Adds new hidable context menu option for ColumnHeaders (#6729)
## Motivation for features / changes Before enabling context menus for scalar card tables, it will be useful to explicitly distinguish the two different removal related column interactions that are currently available: "remove" and "hide". "Removed" columns must be re-added to display again (this is what's currently done for hparams in the runs table). "Hidden" columns can be re-enabled via the "edit table columns" (this is only available in scalar tables). Distinguishing these two operations also provides an easy way to provide similar but different hparam columns for the runs and scalar tables: first add the hparams that you need to both tables, then hide the ones you don't need in the scalar tables. ## Technical description of changes - adds a `hidable` property to the ColumnHeader type. - properly initializes ColumnHeader context menu options in the runs and metrics reducers, and overrides any options set in localstorage. This will effectively turn context options into static, system-defined features. - disables movability and removability for RUN columns. This should have negligible impact on usability while allowing us to define useful column orderings such as `| run | hparam_columns | other_columns |` ## Detailed steps to verify changes work correctly (as executed by you) - Unit tests pass
1 parent 5f778fe commit 189e0f8

File tree

6 files changed

+74
-142
lines changed

6 files changed

+74
-142
lines changed

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

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -275,45 +275,50 @@ const {initialState, reducers: namespaceContextedReducer} =
275275
name: 'run',
276276
displayName: 'Run',
277277
enabled: true,
278-
removable: true,
278+
removable: false,
279279
sortable: true,
280-
movable: true,
280+
movable: false,
281+
hidable: false,
281282
},
282283
{
283284
type: ColumnHeaderType.SMOOTHED,
284285
name: 'smoothed',
285286
displayName: 'Smoothed',
286287
enabled: true,
287-
removable: true,
288+
removable: false,
288289
sortable: true,
289290
movable: true,
291+
hidable: true,
290292
},
291293
{
292294
type: ColumnHeaderType.VALUE,
293295
name: 'value',
294296
displayName: 'Value',
295297
enabled: true,
296-
removable: true,
298+
removable: false,
297299
sortable: true,
298300
movable: true,
301+
hidable: true,
299302
},
300303
{
301304
type: ColumnHeaderType.STEP,
302305
name: 'step',
303306
displayName: 'Step',
304307
enabled: true,
305-
removable: true,
308+
removable: false,
306309
sortable: true,
307310
movable: true,
311+
hidable: true,
308312
},
309313
{
310314
type: ColumnHeaderType.RELATIVE_TIME,
311315
name: 'relative',
312316
displayName: 'Relative',
313317
enabled: true,
314-
removable: true,
318+
removable: false,
315319
sortable: true,
316320
movable: true,
321+
hidable: true,
317322
},
318323
],
319324
rangeSelectionHeaders: [
@@ -322,117 +327,130 @@ const {initialState, reducers: namespaceContextedReducer} =
322327
name: 'run',
323328
displayName: 'Run',
324329
enabled: true,
325-
removable: true,
330+
removable: false,
326331
sortable: true,
327332
movable: true,
333+
hidable: true,
328334
},
329335
{
330336
type: ColumnHeaderType.MIN_VALUE,
331337
name: 'min',
332338
displayName: 'Min',
333339
enabled: true,
334-
removable: true,
340+
removable: false,
335341
sortable: true,
336342
movable: true,
343+
hidable: true,
337344
},
338345
{
339346
type: ColumnHeaderType.MAX_VALUE,
340347
name: 'max',
341348
displayName: 'Max',
342349
enabled: true,
343-
removable: true,
350+
removable: false,
344351
sortable: true,
345352
movable: true,
353+
hidable: true,
346354
},
347355
{
348356
type: ColumnHeaderType.START_VALUE,
349357
name: 'start',
350358
displayName: 'Start Value',
351359
enabled: true,
352-
removable: true,
360+
removable: false,
353361
sortable: true,
354362
movable: true,
363+
hidable: true,
355364
},
356365
{
357366
type: ColumnHeaderType.END_VALUE,
358367
name: 'end',
359368
displayName: 'End Value',
360369
enabled: true,
361-
removable: true,
370+
removable: false,
362371
sortable: true,
363372
movable: true,
373+
hidable: true,
364374
},
365375
{
366376
type: ColumnHeaderType.VALUE_CHANGE,
367377
name: 'valueChange',
368378
displayName: 'Value',
369379
enabled: true,
370-
removable: true,
380+
removable: false,
371381
sortable: true,
372382
movable: true,
383+
hidable: true,
373384
},
374385
{
375386
type: ColumnHeaderType.PERCENTAGE_CHANGE,
376387
name: 'percentageChange',
377388
displayName: '%',
378389
enabled: true,
379-
removable: true,
390+
removable: false,
380391
sortable: true,
381392
movable: true,
393+
hidable: true,
382394
},
383395
{
384396
type: ColumnHeaderType.START_STEP,
385397
name: 'startStep',
386398
displayName: 'Start Step',
387399
enabled: true,
388-
removable: true,
400+
removable: false,
389401
sortable: true,
390402
movable: true,
403+
hidable: true,
391404
},
392405
{
393406
type: ColumnHeaderType.END_STEP,
394407
name: 'endStep',
395408
displayName: 'End Step',
396409
enabled: true,
397-
removable: true,
410+
removable: false,
398411
sortable: true,
399412
movable: true,
413+
hidable: true,
400414
},
401415
{
402416
type: ColumnHeaderType.STEP_AT_MAX,
403417
name: 'stepAtMax',
404418
displayName: 'Step At Max',
405419
enabled: false,
406-
removable: true,
420+
removable: false,
407421
sortable: true,
408422
movable: true,
423+
hidable: true,
409424
},
410425
{
411426
type: ColumnHeaderType.STEP_AT_MIN,
412427
name: 'stepAtMin',
413428
displayName: 'Step At Min',
414429
enabled: false,
415-
removable: true,
430+
removable: false,
416431
sortable: true,
417432
movable: true,
433+
hidable: true,
418434
},
419435
{
420436
type: ColumnHeaderType.MEAN,
421437
name: 'mean',
422438
displayName: 'Mean',
423439
enabled: false,
424-
removable: true,
440+
removable: false,
425441
sortable: true,
426442
movable: true,
443+
hidable: true,
427444
},
428445
{
429446
type: ColumnHeaderType.RAW_CHANGE,
430447
name: 'rawChange',
431448
displayName: 'Raw',
432449
enabled: false,
433-
removable: true,
450+
removable: false,
434451
sortable: true,
435452
movable: true,
453+
hidable: true,
436454
},
437455
],
438456
filteredPluginTypes: new Set(),

‎tensorboard/webapp/persistent_settings/_data_source/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ tf_ng_module(
1111
deps = [
1212
":types",
1313
"//tensorboard/webapp/metrics:types",
14+
"//tensorboard/webapp/widgets/data_table:types",
1415
"@npm//@angular/core",
1516
"@npm//@ngrx/store",
1617
"@npm//rxjs",

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,32 @@ limitations under the License.
1515
import {Injectable} from '@angular/core';
1616
import {EMPTY, Observable, of} from 'rxjs';
1717
import {map, tap} from 'rxjs/operators';
18+
import {ColumnHeader} from '../../widgets/data_table/types';
1819
import {BackendSettings, PersistableSettings, ThemeValue} from './types';
1920

2021
const LEGACY_METRICS_LOCAL_STORAGE_KEY = '_tb_global_settings.timeseries';
2122
const GLOBAL_LOCAL_STORAGE_KEY = '_tb_global_settings';
2223
const NOTIFICATION_LAST_READ_TIME_KEY = 'notificationLastReadTimestamp';
2324

25+
/** Updates context menu related properties for given headers.
26+
*
27+
* Useful for correcting properties that were saved to backend in a possibly inconsistent state.
28+
*/
29+
function updateScalarContextMenuOptions(headers: ColumnHeader[]) {
30+
headers.forEach((header) => {
31+
header.sortable = true;
32+
header.removable = false;
33+
34+
if (header.type === 'RUN') {
35+
header.movable = false;
36+
header.hidable = false;
37+
} else {
38+
header.movable = true;
39+
header.hidable = true;
40+
}
41+
});
42+
}
43+
2444
@Injectable()
2545
export abstract class PersistentSettingsDataSource<UiSettings> {
2646
abstract setSettings(partialSetting: Partial<UiSettings>): Observable<void>;
@@ -217,12 +237,7 @@ export class OSSSettingsConverter extends SettingsConverter<
217237
// If the settings stored in the backend are invalid, reset back to default.
218238
backendSettings.singleSelectionHeaders[0].name !== undefined
219239
) {
220-
// Default these properies to true if stored headers are missing them.
221-
backendSettings.singleSelectionHeaders.forEach((header) => {
222-
header.movable = header.movable ?? true;
223-
header.sortable = header.sortable ?? true;
224-
header.removable = header.removable ?? true;
225-
});
240+
updateScalarContextMenuOptions(backendSettings.singleSelectionHeaders);
226241
settings.singleSelectionHeaders = backendSettings.singleSelectionHeaders;
227242
}
228243

@@ -231,12 +246,7 @@ export class OSSSettingsConverter extends SettingsConverter<
231246
// If the settings stored in the backend are invalid, reset back to default.
232247
backendSettings.rangeSelectionHeaders[0].name !== undefined
233248
) {
234-
// Default these properies to true if stored headers are missing them.
235-
backendSettings.rangeSelectionHeaders.forEach((header) => {
236-
header.movable = header.movable ?? true;
237-
header.sortable = header.sortable ?? true;
238-
header.removable = header.removable ?? true;
239-
});
249+
updateScalarContextMenuOptions(backendSettings.rangeSelectionHeaders);
240250
settings.rangeSelectionHeaders = backendSettings.rangeSelectionHeaders;
241251
}
242252

0 commit comments

Comments
 (0)