diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts b/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts index 105fa2b748..1bd0634288 100644 --- a/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts +++ b/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts @@ -110,6 +110,11 @@ export const FeatureFlagMetadataMap: FeatureFlagMetadataMapType = queryParamOverride: 'enableScalarColumnCustomization', parseValue: parseBoolean, }, + enableScalarColumnContextMenus: { + defaultValue: false, + queryParamOverride: 'enableScalarColumnContextMenus', + parseValue: parseBoolean, + }, enableHparamsInTimeSeries: { defaultValue: true, queryParamOverride: 'enableHparamsInTimeSeries', diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts b/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts index 3c368067be..a6214b5ce5 100644 --- a/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts +++ b/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts @@ -147,6 +147,13 @@ export const getIsScalarColumnCustomizationEnabled = createSelector( } ); +export const getIsScalarColumnContextMenusEnabled = createSelector( + getFeatureFlags, + (flags: FeatureFlags): boolean => { + return flags.enableScalarColumnContextMenus; + } +); + export const getEnableHparamsInTimeSeries = createSelector( getFeatureFlags, (flags: FeatureFlags): boolean => { diff --git a/tensorboard/webapp/feature_flag/types.ts b/tensorboard/webapp/feature_flag/types.ts index f2ec7fc4f3..c6323762cb 100644 --- a/tensorboard/webapp/feature_flag/types.ts +++ b/tensorboard/webapp/feature_flag/types.ts @@ -45,6 +45,8 @@ export interface FeatureFlags { // Adds affordance for users to select and reorder the columns in the Scalar // Card Data Table enableScalarColumnCustomization: boolean; + // Allows users to manipulate Scalar Card Table columns using context menus. + enableScalarColumnContextMenus: boolean; // Adds hparam columns to the runs table and the scalar card data table. enableHparamsInTimeSeries: boolean; // Adds a new section at the top of the time series metrics view diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 899c15f20e..27f86f035b 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -275,47 +275,42 @@ const {initialState, reducers: namespaceContextedReducer} = removable: false, sortable: true, movable: false, - hidable: false, }, { type: ColumnHeaderType.SMOOTHED, name: 'smoothed', displayName: 'Smoothed', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.VALUE, name: 'value', displayName: 'Value', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.STEP, name: 'step', displayName: 'Step', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.RELATIVE_TIME, name: 'relative', displayName: 'Relative', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, ], rangeSelectionHeaders: [ @@ -327,127 +322,114 @@ const {initialState, reducers: namespaceContextedReducer} = removable: false, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.MIN_VALUE, name: 'min', displayName: 'Min', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.MAX_VALUE, name: 'max', displayName: 'Max', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.START_VALUE, name: 'start', displayName: 'Start Value', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.END_VALUE, name: 'end', displayName: 'End Value', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.VALUE_CHANGE, name: 'valueChange', displayName: 'Value', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.PERCENTAGE_CHANGE, name: 'percentageChange', displayName: '%', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.START_STEP, name: 'startStep', displayName: 'Start Step', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.END_STEP, name: 'endStep', displayName: 'End Step', enabled: true, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.STEP_AT_MAX, name: 'stepAtMax', displayName: 'Step At Max', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.STEP_AT_MIN, name: 'stepAtMin', displayName: 'Step At Min', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.MEAN, name: 'mean', displayName: 'Mean', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, { type: ColumnHeaderType.RAW_CHANGE, name: 'rawChange', displayName: 'Raw', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, ], filteredPluginTypes: new Set(), diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index 8d457246da..b26d5617db 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -198,6 +198,7 @@ [columnHeaders]="columnHeaders" [sortingInfo]="sortingInfo" [columnCustomizationEnabled]="columnCustomizationEnabled" + [columnContextMenusEnabled]="columnContextMenusEnabled" [smoothingEnabled]="smoothingEnabled" [hparamsEnabled]="hparamsEnabled" (sortDataBy)="sortDataBy($event)" diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index 555b47f771..7ea60f216a 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -95,6 +95,7 @@ export class ScalarCardComponent { @Input() useDarkMode!: boolean; @Input() forceSvg!: boolean; @Input() columnCustomizationEnabled!: boolean; + @Input() columnContextMenusEnabled!: boolean; @Input() linkedTimeSelection: TimeSelectionView | undefined; @Input() stepOrLinkedTimeSelection: TimeSelection | undefined; @Input() minMaxStep!: MinMaxStep; diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts index df0c33d611..571707121c 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts @@ -40,6 +40,7 @@ import {ExperimentAlias} from '../../../experiments/types'; import { getEnableHparamsInTimeSeries, getForceSvgFeatureFlag, + getIsScalarColumnContextMenusEnabled, getIsScalarColumnCustomizationEnabled, } from '../../../feature_flag/store/feature_flag_selectors'; import { @@ -169,6 +170,7 @@ function areSeriesEqual( [stepOrLinkedTimeSelection]="stepOrLinkedTimeSelection$ | async" [forceSvg]="forceSvg$ | async" [columnCustomizationEnabled]="columnCustomizationEnabled$ | async" + [columnContextMenusEnabled]="columnContextMenusEnabled$ | async" [minMaxStep]="minMaxSteps$ | async" [userViewBox]="userViewBox$ | async" [columnHeaders]="columnHeaders$ | async" @@ -237,6 +239,9 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { readonly columnCustomizationEnabled$ = this.store.select( getIsScalarColumnCustomizationEnabled ); + readonly columnContextMenusEnabled$ = this.store.select( + getIsScalarColumnContextMenusEnabled + ); readonly xScaleType$ = this.store.select(getMetricsXAxisType).pipe( map((xAxisType) => { switch (xAxisType) { diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html index 902bdb9d1b..c17700e465 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html @@ -28,7 +28,7 @@ [header]="header" [sortingInfo]="sortingInfo" [hparamsEnabled]="hparamsEnabled" - disableContextMenu="true" + [disableContextMenu]="!columnContextMenusEnabled" > diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts index 15e3194bd7..00feae5640 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts @@ -51,6 +51,7 @@ export class ScalarCardDataTable { @Input() columnHeaders!: ColumnHeader[]; @Input() sortingInfo!: SortingInfo; @Input() columnCustomizationEnabled!: boolean; + @Input() columnContextMenusEnabled!: boolean; @Input() smoothingEnabled!: boolean; @Input() hparamsEnabled?: boolean; diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts index caf82d14aa..894c80893c 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -128,6 +128,7 @@ import * as commonSelectors from '../main_view/common_selectors'; import {ContentCellComponent} from '../../../widgets/data_table/content_cell_component'; import {ContentRowComponent} from '../../../widgets/data_table/content_row_component'; import {HeaderCellComponent} from '../../../widgets/data_table/header_cell_component'; +import {getIsScalarColumnContextMenusEnabled} from '../../../selectors'; @Component({ selector: 'line-chart', @@ -385,6 +386,10 @@ describe('scalar card', () => { selectors.getIsScalarColumnCustomizationEnabled, false ); + store.overrideSelector( + selectors.getIsScalarColumnContextMenusEnabled, + false + ); store.overrideSelector(selectors.getMetricsStepSelectorEnabled, false); store.overrideSelector( selectors.getMetricsCardRangeSelectionEnabled('card1'), @@ -4412,6 +4417,56 @@ describe('scalar card', () => { expect(dataTableComponent).toBeFalsy(); })); + it('disables context menus if columnContextMenusEnabled is not set', fakeAsync(() => { + store.overrideSelector(getIsScalarColumnContextMenusEnabled, false); + store.overrideSelector(getCardStateMap, { + card1: { + dataMinMax: { + minStep: 0, + maxStep: 100, + }, + }, + }); + store.overrideSelector(getMetricsCardTimeSelection, { + start: {step: 0}, + end: {step: 100}, + }); + store.overrideSelector(selectors.getMetricsStepSelectorEnabled, true); + const fixture = createComponent('card1'); + fixture.detectChanges(); + + const headerCellComponentInstance = fixture.debugElement.query( + By.directive(HeaderCellComponent) + ).componentInstance; + + expect(headerCellComponentInstance.disableContextMenu).toBeTrue(); + })); + + it('enables context menus if columnContextMenusEnabled is set', fakeAsync(() => { + store.overrideSelector(getIsScalarColumnContextMenusEnabled, true); + store.overrideSelector(getCardStateMap, { + card1: { + dataMinMax: { + minStep: 0, + maxStep: 100, + }, + }, + }); + store.overrideSelector(getMetricsCardTimeSelection, { + start: {step: 0}, + end: {step: 100}, + }); + store.overrideSelector(selectors.getMetricsStepSelectorEnabled, true); + const fixture = createComponent('card1'); + fixture.detectChanges(); + + const headerCellComponentInstance = fixture.debugElement.query( + By.directive(HeaderCellComponent) + ).componentInstance; + + expect(headerCellComponentInstance.disableContextMenu).toBeFalse(); + })); + it('emits dataTableColumnOrderChanged with DataTableMode.SINGLE when orderColumns is called while in Single Selection', fakeAsync(() => { store.overrideSelector(getCardStateMap, { card1: { diff --git a/tensorboard/webapp/metrics/views/main_view/common_selectors.ts b/tensorboard/webapp/metrics/views/main_view/common_selectors.ts index 0836af7fb4..85969710f2 100644 --- a/tensorboard/webapp/metrics/views/main_view/common_selectors.ts +++ b/tensorboard/webapp/metrics/views/main_view/common_selectors.ts @@ -288,7 +288,6 @@ export const getPotentialHparamColumns = createSelector( sortable: true, movable: true, filterable: true, - hidable: true, })); } ); diff --git a/tensorboard/webapp/metrics/views/main_view/common_selectors_test.ts b/tensorboard/webapp/metrics/views/main_view/common_selectors_test.ts index 11b133f54b..72c6dbe092 100644 --- a/tensorboard/webapp/metrics/views/main_view/common_selectors_test.ts +++ b/tensorboard/webapp/metrics/views/main_view/common_selectors_test.ts @@ -172,7 +172,6 @@ describe('common selectors', () => { removable: false, movable: false, filterable: false, - hidable: false, }, { type: ColumnHeaderType.CUSTOM, @@ -1021,7 +1020,6 @@ describe('common selectors', () => { sortable: true, movable: true, filterable: true, - hidable: true, }; it('returns empty list when there are no experiments', () => { diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts index 612756f7ae..432c79c8ff 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts @@ -29,14 +29,13 @@ const NOTIFICATION_LAST_READ_TIME_KEY = 'notificationLastReadTimestamp'; function updateScalarContextMenuOptions(headers: ColumnHeader[]) { headers.forEach((header) => { header.sortable = true; - header.removable = false; if (header.type === 'RUN') { header.movable = false; - header.hidable = false; + header.removable = false; } else { header.movable = true; - header.hidable = true; + header.removable = true; } }); } diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts index 959c02ca83..4c938a169b 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts @@ -215,17 +215,15 @@ describe('persistent_settings data_source test', () => { removable: false, sortable: true, movable: false, - hidable: false, }, { type: ColumnHeaderType.VALUE, name: 'value', displayName: 'Value', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, ], }); @@ -263,17 +261,15 @@ describe('persistent_settings data_source test', () => { removable: false, sortable: true, movable: false, - hidable: false, }, { type: ColumnHeaderType.MIN_VALUE, name: 'minValue', displayName: 'Min', enabled: false, - removable: false, + removable: true, sortable: true, movable: true, - hidable: true, }, ], }); diff --git a/tensorboard/webapp/runs/store/runs_reducers.ts b/tensorboard/webapp/runs/store/runs_reducers.ts index d43c8f799f..9e0c3712db 100644 --- a/tensorboard/webapp/runs/store/runs_reducers.ts +++ b/tensorboard/webapp/runs/store/runs_reducers.ts @@ -324,7 +324,6 @@ const {initialState: uiInitialState, reducers: uiNamespaceContextedReducers} = removable: false, movable: false, filterable: false, - hidable: false, }, ], sortingInfo: { diff --git a/tensorboard/webapp/runs/store/runs_selectors.ts b/tensorboard/webapp/runs/store/runs_selectors.ts index 137c07d618..ef9b7fe204 100644 --- a/tensorboard/webapp/runs/store/runs_selectors.ts +++ b/tensorboard/webapp/runs/store/runs_selectors.ts @@ -330,16 +330,6 @@ export const getRunsTableSortingInfo = createSelector( export const getGroupedRunsTableHeaders = createSelector( getRunsTableHeaders, getDashboardDisplayedHparamColumns, - (runsTableHeaders, hparamColumns) => { - // Override hparam options to match runs table requirements. - const columns = [...runsTableHeaders, ...hparamColumns].map((column) => { - const newColumn = {...column}; - if (column.type === 'HPARAM') { - newColumn.removable = true; - newColumn.hidable = false; - } - return newColumn; - }); - return dataTableUtils.groupColumns(columns); - } + (runsTableHeaders, hparamColumns) => + dataTableUtils.groupColumns([...runsTableHeaders, ...hparamColumns]) ); diff --git a/tensorboard/webapp/runs/store/runs_selectors_test.ts b/tensorboard/webapp/runs/store/runs_selectors_test.ts index c1e083a69c..80651a40ea 100644 --- a/tensorboard/webapp/runs/store/runs_selectors_test.ts +++ b/tensorboard/webapp/runs/store/runs_selectors_test.ts @@ -1120,36 +1120,6 @@ describe('runs_selectors', () => { }), ]); }); - - it('sets the hparam column context options for the runs table', () => { - expect(selectors.getGroupedRunsTableHeaders(state)).toEqual([ - jasmine.objectContaining({ - type: ColumnHeaderType.RUN, - }), - jasmine.objectContaining({ - type: ColumnHeaderType.CUSTOM, - }), - jasmine.objectContaining({ - type: ColumnHeaderType.HPARAM, - name: 'conv_layers', - displayName: 'Conv Layers', - enabled: true, - removable: true, - hidable: false, - }), - jasmine.objectContaining({ - type: ColumnHeaderType.HPARAM, - name: 'conv_kernel_size', - displayName: 'Conv Kernel Size', - enabled: true, - removable: true, - hidable: false, - }), - jasmine.objectContaining({ - type: ColumnHeaderType.COLOR, - }), - ]); - }); }); describe('#getRunsTableSortingInfo', () => { diff --git a/tensorboard/webapp/widgets/data_table/types.ts b/tensorboard/webapp/widgets/data_table/types.ts index 38607a2630..8c61bb6a67 100644 --- a/tensorboard/webapp/widgets/data_table/types.ts +++ b/tensorboard/webapp/widgets/data_table/types.ts @@ -86,7 +86,6 @@ export declare interface ColumnHeader { sortable?: boolean; movable?: boolean; filterable?: boolean; - hidable?: boolean; } export enum SortingOrder {