diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index 3cd3ebc823..a7f8f52d10 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -240,7 +240,7 @@ export const dataTableColumnEdited = createAction( ); export const dataTableColumnToggled = createAction( - '[Metrics] Data table column toggled in edit menu', + '[Metrics] Data table column toggled in edit menu or delete button clicked', props() ); diff --git a/tensorboard/webapp/metrics/internal_types.ts b/tensorboard/webapp/metrics/internal_types.ts index 57ce7b8156..c5cc4617e4 100644 --- a/tensorboard/webapp/metrics/internal_types.ts +++ b/tensorboard/webapp/metrics/internal_types.ts @@ -100,8 +100,9 @@ export interface HeaderEditInfo { } export interface HeaderToggleInfo { - dataTableMode: DataTableMode; headerType: ColumnHeaderType; + cardId?: CardId; + dataTableMode?: DataTableMode; } export const SCALARS_SMOOTHING_MIN = 0; diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 05b84c9ae0..8e3929a636 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -55,6 +55,7 @@ import { buildOrReturnStateWithPinnedCopy, buildOrReturnStateWithUnresolvedImportedPins, canCreateNewPins, + cardRangeSelectionEnabled, createPluginDataWithLoadable, createRunToLoadState, generateNextCardStepIndex, @@ -1372,46 +1373,58 @@ const reducer = createReducer( singleSelectionHeaders: enabledNewHeaders.concat(disabledNewHeaders), }; }), - on(actions.dataTableColumnToggled, (state, {dataTableMode, headerType}) => { - const targetedHeaders = - dataTableMode === DataTableMode.RANGE + on( + actions.dataTableColumnToggled, + (state, {dataTableMode, headerType, cardId}) => { + const {cardStateMap, rangeSelectionEnabled, linkedTimeEnabled} = state; + const rangeEnabled = cardId + ? cardRangeSelectionEnabled( + cardStateMap, + rangeSelectionEnabled, + linkedTimeEnabled, + cardId + ) + : dataTableMode === DataTableMode.RANGE; + + const targetedHeaders = rangeEnabled ? state.rangeSelectionHeaders : state.singleSelectionHeaders; - const currentToggledHeaderIndex = targetedHeaders.findIndex( - (element) => element.type === headerType - ); + const currentToggledHeaderIndex = targetedHeaders.findIndex( + (element) => element.type === headerType + ); - // If the header is being enabled it goes at the bottom of the currently - // enabled headers. If it is being disabled it goes to the top of the - // currently disabled headers. - let newToggledHeaderIndex = getEnabledCount(targetedHeaders); - if (targetedHeaders[currentToggledHeaderIndex].enabled) { - newToggledHeaderIndex--; - } - const newHeaders = moveHeader( - currentToggledHeaderIndex, - newToggledHeaderIndex, - targetedHeaders - ); + // If the header is being enabled it goes at the bottom of the currently + // enabled headers. If it is being disabled it goes to the top of the + // currently disabled headers. + let newToggledHeaderIndex = getEnabledCount(targetedHeaders); + if (targetedHeaders[currentToggledHeaderIndex].enabled) { + newToggledHeaderIndex--; + } + const newHeaders = moveHeader( + currentToggledHeaderIndex, + newToggledHeaderIndex, + targetedHeaders + ); - newHeaders[newToggledHeaderIndex] = { - ...newHeaders[newToggledHeaderIndex], - enabled: !newHeaders[newToggledHeaderIndex].enabled, - }; + newHeaders[newToggledHeaderIndex] = { + ...newHeaders[newToggledHeaderIndex], + enabled: !newHeaders[newToggledHeaderIndex].enabled, + }; + + if (rangeEnabled) { + return { + ...state, + rangeSelectionHeaders: newHeaders, + }; + } - if (dataTableMode === DataTableMode.RANGE) { return { ...state, - rangeSelectionHeaders: newHeaders, + singleSelectionHeaders: newHeaders, }; } - - return { - ...state, - singleSelectionHeaders: newHeaders, - }; - }), + ), on(actions.metricsToggleVisiblePlugin, (state, {plugin}) => { let nextFilteredPluginTypes = new Set(state.filteredPluginTypes); if (nextFilteredPluginTypes.has(plugin)) { diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index cd81c60377..55f927ccd7 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -1964,8 +1964,10 @@ describe('metrics reducers', () => { }); describe('dataTableColumnToggled', () => { - it('moves header down to the disabled headers when toggling to disabled', () => { - const beforeState = buildMetricsState({ + let beforeState: MetricsState; + + beforeEach(() => { + beforeState = buildMetricsState({ rangeSelectionHeaders: [ { type: ColumnHeaderType.RUN, @@ -1998,8 +2000,36 @@ describe('metrics reducers', () => { enabled: false, }, ], + singleSelectionHeaders: [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.VALUE, + name: 'value', + displayName: 'Value', + enabled: true, + }, + { + type: ColumnHeaderType.STEP, + name: 'step', + displayName: 'Step', + enabled: true, + }, + { + type: ColumnHeaderType.RELATIVE_TIME, + name: 'relativeTime', + displayName: 'Relative', + enabled: false, + }, + ], }); + }); + it('moves header down to the disabled headers when toggling to disabled with data table mode input', () => { const nextState = reducers( beforeState, actions.dataTableColumnToggled({ @@ -2042,42 +2072,7 @@ describe('metrics reducers', () => { ]); }); - it('moves header up to the enabled headers when toggling to enabled', () => { - const beforeState = buildMetricsState({ - rangeSelectionHeaders: [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - { - type: ColumnHeaderType.START_VALUE, - name: 'startValue', - displayName: 'Start Value', - enabled: true, - }, - { - type: ColumnHeaderType.END_VALUE, - name: 'endValue', - displayName: 'End Value', - enabled: true, - }, - { - type: ColumnHeaderType.MIN_VALUE, - name: 'minValue', - displayName: 'Min', - enabled: false, - }, - { - type: ColumnHeaderType.MAX_VALUE, - name: 'maxValue', - displayName: 'Max', - enabled: false, - }, - ], - }); - + it('moves header up to the enabled headers when toggling to enabled with data table mode input', () => { const nextState = reducers( beforeState, actions.dataTableColumnToggled({ @@ -2121,67 +2116,6 @@ describe('metrics reducers', () => { }); it('only changes range selection headers when dataTableMode is RANGE', () => { - const beforeState = buildMetricsState({ - rangeSelectionHeaders: [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - { - type: ColumnHeaderType.START_VALUE, - name: 'startValue', - displayName: 'Start Value', - enabled: true, - }, - { - type: ColumnHeaderType.END_VALUE, - name: 'endValue', - displayName: 'End Value', - enabled: true, - }, - { - type: ColumnHeaderType.MIN_VALUE, - name: 'minValue', - displayName: 'Min', - enabled: false, - }, - { - type: ColumnHeaderType.MAX_VALUE, - name: 'maxValue', - displayName: 'Max', - enabled: false, - }, - ], - singleSelectionHeaders: [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - { - type: ColumnHeaderType.STEP, - name: 'step', - displayName: 'Step', - enabled: true, - }, - { - type: ColumnHeaderType.VALUE, - name: 'value', - displayName: 'Value', - enabled: true, - }, - { - type: ColumnHeaderType.RELATIVE_TIME, - name: 'relativeTime', - displayName: 'Relative', - enabled: false, - }, - ], - }); - const nextState = reducers( beforeState, actions.dataTableColumnToggled({ @@ -2222,6 +2156,7 @@ describe('metrics reducers', () => { enabled: false, }, ]); + expect(nextState.singleSelectionHeaders).toEqual([ { type: ColumnHeaderType.RUN, @@ -2229,18 +2164,88 @@ describe('metrics reducers', () => { displayName: 'Run', enabled: true, }, + { + type: ColumnHeaderType.VALUE, + name: 'value', + displayName: 'Value', + enabled: true, + }, { type: ColumnHeaderType.STEP, name: 'step', displayName: 'Step', enabled: true, }, + { + type: ColumnHeaderType.RELATIVE_TIME, + name: 'relativeTime', + displayName: 'Relative', + enabled: false, + }, + ]); + }); + + it('only changes single selection headers when dataTableMode is SINGLE', () => { + const nextState = reducers( + beforeState, + actions.dataTableColumnToggled({ + dataTableMode: DataTableMode.SINGLE, + headerType: ColumnHeaderType.STEP, + }) + ); + + expect(nextState.rangeSelectionHeaders).toEqual([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.START_VALUE, + name: 'startValue', + displayName: 'Start Value', + enabled: true, + }, + { + type: ColumnHeaderType.END_VALUE, + name: 'endValue', + displayName: 'End Value', + enabled: true, + }, + { + type: ColumnHeaderType.MIN_VALUE, + name: 'minValue', + displayName: 'Min', + enabled: false, + }, + { + type: ColumnHeaderType.MAX_VALUE, + name: 'maxValue', + displayName: 'Max', + enabled: false, + }, + ]); + + expect(nextState.singleSelectionHeaders).toEqual([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, { type: ColumnHeaderType.VALUE, name: 'value', displayName: 'Value', enabled: true, }, + { + type: ColumnHeaderType.STEP, + name: 'step', + displayName: 'Step', + enabled: false, + }, { type: ColumnHeaderType.RELATIVE_TIME, name: 'relativeTime', @@ -2250,72 +2255,122 @@ describe('metrics reducers', () => { ]); }); - it('only changes single selection headers when dataTableMode is SINGLE', () => { - const beforeState = buildMetricsState({ - rangeSelectionHeaders: [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - { - type: ColumnHeaderType.START_VALUE, - name: 'startValue', - displayName: 'Start Value', - enabled: true, - }, - { - type: ColumnHeaderType.END_VALUE, - name: 'endValue', - displayName: 'End Value', - enabled: true, - }, - { - type: ColumnHeaderType.MIN_VALUE, - name: 'minValue', - displayName: 'Min', - enabled: false, - }, - { - type: ColumnHeaderType.MAX_VALUE, - name: 'maxValue', - displayName: 'Max', - enabled: false, - }, - ], - singleSelectionHeaders: [ - { - type: ColumnHeaderType.RUN, - name: 'run', - displayName: 'Run', - enabled: true, - }, - { - type: ColumnHeaderType.STEP, - name: 'step', - displayName: 'Step', - enabled: true, + it('moves header down to the disabled headers when column is removed with card id input', () => { + beforeState = { + ...beforeState, + cardStateMap: { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, }, - { - type: ColumnHeaderType.VALUE, - name: 'value', - displayName: 'Value', - enabled: true, + }, + }; + + const nextState = reducers( + beforeState, + actions.dataTableColumnToggled({ + cardId: 'card1', + headerType: ColumnHeaderType.RUN, + }) + ); + + expect( + nextState.rangeSelectionHeaders.map((header) => header.enabled) + ).toEqual([true, true, false, false, false]); + }); + + it('only changes range selection headers when given card has rangeSelectionOverride ENABLED', () => { + beforeState = { + ...beforeState, + cardStateMap: { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, }, - { - type: ColumnHeaderType.RELATIVE_TIME, - name: 'relativeTime', - displayName: 'Relative', - enabled: false, + }, + }; + + const nextState = reducers( + beforeState, + actions.dataTableColumnToggled({ + cardId: 'card1', + headerType: ColumnHeaderType.MAX_VALUE, + }) + ); + + expect(nextState.rangeSelectionHeaders).toEqual([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.START_VALUE, + name: 'startValue', + displayName: 'Start Value', + enabled: true, + }, + { + type: ColumnHeaderType.END_VALUE, + name: 'endValue', + displayName: 'End Value', + enabled: true, + }, + { + type: ColumnHeaderType.MAX_VALUE, + name: 'maxValue', + displayName: 'Max', + enabled: true, + }, + { + type: ColumnHeaderType.MIN_VALUE, + name: 'minValue', + displayName: 'Min', + enabled: false, + }, + ]); + + expect(nextState.singleSelectionHeaders).toEqual([ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.VALUE, + name: 'value', + displayName: 'Value', + enabled: true, + }, + { + type: ColumnHeaderType.STEP, + name: 'step', + displayName: 'Step', + enabled: true, + }, + { + type: ColumnHeaderType.RELATIVE_TIME, + name: 'relativeTime', + displayName: 'Relative', + enabled: false, + }, + ]); + }); + + it('only changes single selection headers when given card has rangeSelectionOverride DISABLED', () => { + beforeState = { + ...beforeState, + cardStateMap: { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, }, - ], - }); + }, + }; const nextState = reducers( beforeState, actions.dataTableColumnToggled({ - dataTableMode: DataTableMode.SINGLE, + cardId: 'card1', headerType: ColumnHeaderType.STEP, }) ); @@ -2352,6 +2407,7 @@ describe('metrics reducers', () => { enabled: false, }, ]); + expect(nextState.singleSelectionHeaders).toEqual([ { type: ColumnHeaderType.RUN, diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index ffb705aa76..0e35780e6d 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -33,6 +33,7 @@ import {MinMaxStep} from '../views/card_renderer/scalar_card_types'; import {formatTimeSelection} from '../views/card_renderer/utils'; import * as storeUtils from './metrics_store_internal_utils'; import { + cardRangeSelectionEnabled, getCardSelectionStateToBoolean, getMinMaxStepFromCardState, } from './metrics_store_internal_utils'; @@ -476,25 +477,21 @@ export const getTableEditorSelectedTab = createSelector( ); export const getMetricsCardRangeSelectionEnabled = createSelector( + getCardStateMap, getMetricsRangeSelectionEnabled, getMetricsLinkedTimeEnabled, - getCardStateMap, ( + cardStateMap: CardStateMap, globalRangeSelectionEnabled: boolean, linkedTimeEnabled: boolean, - cardStateMap: CardStateMap, cardId: CardId - ) => { - if (linkedTimeEnabled) { - return globalRangeSelectionEnabled; - } - - const cardState = cardStateMap[cardId]; - return getCardSelectionStateToBoolean( - cardState?.rangeSelectionOverride, - globalRangeSelectionEnabled - ); - } + ) => + cardRangeSelectionEnabled( + cardStateMap, + globalRangeSelectionEnabled, + linkedTimeEnabled, + cardId + ) ); /** diff --git a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts index efd57e56bc..d1b3414093 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils.ts @@ -601,6 +601,23 @@ export function getCardSelectionStateToBoolean( } } +export function cardRangeSelectionEnabled( + cardStateMap: CardStateMap, + globalRangeSelectionEnabled: boolean, + linkedTimeEnabled: boolean, + cardId: CardId +): boolean { + if (linkedTimeEnabled) { + return globalRangeSelectionEnabled; + } + + const cardState = cardStateMap[cardId]; + return getCardSelectionStateToBoolean( + cardState?.rangeSelectionOverride, + globalRangeSelectionEnabled + ); +} + export const TEST_ONLY = { getImageCardSteps, getSelectedSteps, diff --git a/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts b/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts index 382e02f02c..5d24c8433d 100644 --- a/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_store_internal_utils_test.ts @@ -25,6 +25,7 @@ import { buildOrReturnStateWithPinnedCopy, buildOrReturnStateWithUnresolvedImportedPins, canCreateNewPins, + cardRangeSelectionEnabled, createPluginDataWithLoadable, createRunToLoadState, generateNextCardStepIndex, @@ -1278,4 +1279,86 @@ describe('metrics store utils', () => { expect(getCardSelectionStateToBoolean(undefined, false)).toBeFalse(); }); }); + + describe('cardRangeSelectionEnabled', () => { + it('returns card specific value when defined', () => { + expect( + cardRangeSelectionEnabled( + { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, + }, + }, + false, + false, + 'card1' + ) + ).toBeTrue(); + + expect( + cardRangeSelectionEnabled( + { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, + }, + }, + true, + false, + 'card1' + ) + ).toBeFalse(); + }); + + it('returns global value when card specific value is not defined', () => { + expect( + cardRangeSelectionEnabled( + { + card1: {}, + }, + true, + false, + 'card1' + ) + ).toBeTrue(); + + expect( + cardRangeSelectionEnabled( + { + card1: {}, + }, + false, + false, + 'card1' + ) + ).toBeFalse(); + }); + + it('returns global value when linked time is enabled', () => { + expect( + cardRangeSelectionEnabled( + { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, + }, + }, + true, + true, + 'card1' + ) + ).toBeTrue(); + + expect( + cardRangeSelectionEnabled( + { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, + }, + }, + false, + true, + 'card1' + ) + ).toBeFalse(); + }); + }); }); diff --git a/tensorboard/webapp/metrics/views/card_renderer/BUILD b/tensorboard/webapp/metrics/views/card_renderer/BUILD index d86a65d8e7..2a5debd900 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/BUILD +++ b/tensorboard/webapp/metrics/views/card_renderer/BUILD @@ -382,6 +382,7 @@ tf_ts_library( "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/metrics/store", + "//tensorboard/webapp/metrics/store:types", "//tensorboard/webapp/metrics/views/main_view:common_selectors", "//tensorboard/webapp/runs/store:testing", "//tensorboard/webapp/runs/store:types", 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 b6574e4973..48e88a11ec 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 @@ -200,8 +200,10 @@ [sortingInfo]="sortingInfo" [columnCustomizationEnabled]="columnCustomizationEnabled" [smoothingEnabled]="smoothingEnabled" + [hparamsEnabled]="hparamsEnabled" (sortDataBy)="sortDataBy($event)" (editColumnHeaders)="editColumnHeaders.emit($event)" + (removeColumn)="removeColumn($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 7f827a6f82..f1dd1c177b 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -44,7 +44,12 @@ import { TooltipDatum, } from '../../../widgets/line_chart_v2/types'; import {CardState} from '../../store'; -import {HeaderEditInfo, TooltipSort, XAxisType} from '../../types'; +import { + HeaderEditInfo, + HeaderToggleInfo, + TooltipSort, + XAxisType, +} from '../../types'; import { MinMaxStep, ScalarCardDataSeries, @@ -102,6 +107,7 @@ export class ScalarCardComponent { @Input() userViewBox!: Extent | null; @Input() columnHeaders!: ColumnHeader[]; @Input() rangeEnabled!: boolean; + @Input() hparamsEnabled?: boolean; @Output() onFullSizeToggle = new EventEmitter(); @Output() onPinClicked = new EventEmitter(); @@ -114,6 +120,7 @@ export class ScalarCardComponent { @Output() onDataTableSorting = new EventEmitter(); @Output() editColumnHeaders = new EventEmitter(); @Output() openTableEditMenuToMode = new EventEmitter(); + @Output() onRemoveColumn = new EventEmitter(); @Output() onLineChartZoom = new EventEmitter(); @@ -145,6 +152,13 @@ export class ScalarCardComponent { this.onDataTableSorting.emit(sortingInfo); } + removeColumn(headerToggleInfo: HeaderToggleInfo) { + this.onRemoveColumn.emit({ + headerType: headerToggleInfo.headerType, + cardId: this.cardId, + }); + } + resetDomain() { if (this.lineChart) { this.lineChart.viewBoxReset(); 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 848865fd5f..0ffa0a92e8 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts @@ -71,6 +71,7 @@ import {ScaleType} from '../../../widgets/line_chart_v2/types'; import { cardViewBoxChanged, dataTableColumnEdited, + dataTableColumnToggled, metricsCardFullSizeToggled, metricsCardStateUpdated, sortingDataTable, @@ -92,7 +93,13 @@ import { getMetricsXAxisType, RunToSeries, } from '../../store'; -import {CardId, CardMetadata, HeaderEditInfo, XAxisType} from '../../types'; +import { + CardId, + CardMetadata, + HeaderEditInfo, + HeaderToggleInfo, + XAxisType, +} from '../../types'; import {getFilteredRenderableRunsIdsFromRoute} from '../main_view/common_selectors'; import {CardRenderer} from '../metrics_view_types'; import {getTagDisplayName} from '../utils'; @@ -177,6 +184,7 @@ function isMinMaxStepValid(minMax: MinMaxStep | undefined): boolean { [userViewBox]="userViewBox$ | async" [columnHeaders]="columnHeaders$ | async" [rangeEnabled]="rangeEnabled$ | async" + [hparamsEnabled]="hparamsEnabled$ | async" (onFullSizeToggle)="onFullSizeToggle()" (onPinClicked)="pinStateChanged.emit($event)" observeIntersection @@ -188,6 +196,7 @@ function isMinMaxStepValid(minMax: MinMaxStep | undefined): boolean { (editColumnHeaders)="editColumnHeaders($event)" (onCardStateChanged)="onCardStateChanged($event)" (openTableEditMenuToMode)="openTableEditMenuToMode($event)" + (onRemoveColumn)="onRemoveColumn($event)" > `, styles: [ @@ -226,6 +235,7 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { stepOrLinkedTimeSelection$?: Observable; cardState$?: Observable>; rangeEnabled$?: Observable; + hparamsEnabled$?: Observable; onVisibilityChange({visible}: {visible: boolean}) { this.isVisible = visible; @@ -589,6 +599,8 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { getMetricsCardRangeSelectionEnabled, this.cardId ); + + this.hparamsEnabled$ = this.store.select(getEnableHparamsInTimeSeries); } ngOnDestroy() { @@ -682,4 +694,8 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { openTableEditMenuToMode(tableMode: DataTableMode) { this.store.dispatch(metricsSlideoutMenuOpened({mode: tableMode})); } + + onRemoveColumn(headerToggleInfo: HeaderToggleInfo) { + this.store.dispatch(dataTableColumnToggled(headerToggleInfo)); + } } 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 87447b9385..410bb057dc 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 @@ -48,6 +48,7 @@ import {isDatumVisible} from './utils'; [smoothingEnabled]="smoothingEnabled" (sortDataBy)="sortDataBy.emit($event)" (orderColumns)="orderColumns($event)" + (removeColumn)="removeColumn.emit($event)" > @@ -55,6 +56,7 @@ import {isDatumVisible} from './utils'; *ngIf="header.enabled" [header]="header" [sortingInfo]="sortingInfo" + [hparamsEnabled]="hparamsEnabled" > @@ -69,9 +71,13 @@ export class ScalarCardDataTable { @Input() sortingInfo!: SortingInfo; @Input() columnCustomizationEnabled!: boolean; @Input() smoothingEnabled!: boolean; + @Input() hparamsEnabled?: boolean; @Output() sortDataBy = new EventEmitter(); @Output() editColumnHeaders = new EventEmitter(); + @Output() removeColumn = new EventEmitter<{ + headerType: ColumnHeaderType; + }>(); getMinPointInRange( points: ScalarCardPoint[], 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 c133e43cdc..02c44216ce 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -82,6 +82,7 @@ import { timeSelectionChanged, metricsSlideoutMenuOpened, dataTableColumnEdited, + dataTableColumnToggled, } from '../../actions'; import {PluginType} from '../../data_source'; import { @@ -122,6 +123,7 @@ import {VisLinkedTimeSelectionWarningModule} from './vis_linked_time_selection_w import {Extent} from '../../../widgets/line_chart_v2/lib/public_types'; import {provideMockTbStore} from '../../../testing/utils'; import * as commonSelectors from '../main_view/common_selectors'; +import {CardFeatureOverride} from '../../store/metrics_types'; @Component({ selector: 'line-chart', @@ -4305,6 +4307,78 @@ describe('scalar card', () => { }), ]); })); + + it('emits dataTableColumnToggled when onRemoveColumn is called with range selection disabled', fakeAsync(() => { + store.overrideSelector(getSingleSelectionHeaders, [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.VALUE, + name: 'value', + displayName: 'Value', + enabled: false, + }, + ]); + store.overrideSelector(getCardStateMap, { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_DISABLED, + }, + }); + const fixture = createComponent('card1'); + fixture.detectChanges(); + + fixture.componentInstance.onRemoveColumn({ + cardId: 'card1', + headerType: ColumnHeaderType.RUN, + }); + + expect(dispatchedActions).toEqual([ + dataTableColumnToggled({ + cardId: 'card1', + headerType: ColumnHeaderType.RUN, + }), + ]); + })); + + it('emits dataTableColumnToggled when onRemoveColumn is called with range selection enabled', fakeAsync(() => { + store.overrideSelector(getRangeSelectionHeaders, [ + { + type: ColumnHeaderType.RUN, + name: 'run', + displayName: 'Run', + enabled: true, + }, + { + type: ColumnHeaderType.MIN_VALUE, + name: 'minValue', + displayName: 'Min Value', + enabled: true, + }, + ]); + store.overrideSelector(getCardStateMap, { + card1: { + rangeSelectionOverride: CardFeatureOverride.OVERRIDE_AS_ENABLED, + }, + }); + const fixture = createComponent('card1'); + fixture.detectChanges(); + + fixture.componentInstance.onRemoveColumn({ + cardId: 'card1', + headerType: ColumnHeaderType.MIN_VALUE, + }); + + expect(dispatchedActions).toEqual([ + dataTableColumnToggled({ + cardId: 'card1', + headerType: ColumnHeaderType.MIN_VALUE, + }), + ]); + })); }); }); diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ts b/tensorboard/webapp/widgets/data_table/data_table_component.ts index 12066e6de4..47caed0687 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_component.ts +++ b/tensorboard/webapp/widgets/data_table/data_table_component.ts @@ -69,6 +69,9 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { @Output() sortDataBy = new EventEmitter(); @Output() orderColumns = new EventEmitter(); + @Output() removeColumn = new EventEmitter<{ + headerType: ColumnHeaderType; + }>(); readonly ColumnHeaders = ColumnHeaderType; readonly SortingOrder = SortingOrder; @@ -100,7 +103,10 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { headerCell.dragStart.subscribe(this.dragStart.bind(this)), headerCell.dragEnter.subscribe(this.dragEnter.bind(this)), headerCell.dragEnd.subscribe(this.dragEnd.bind(this)), - headerCell.headerClicked.subscribe(this.headerClicked.bind(this)) + headerCell.headerClicked.subscribe(this.headerClicked.bind(this)), + headerCell.deleteButtonClicked.subscribe( + this.deleteButtonClicked.bind(this) + ) ); }); } @@ -246,4 +252,10 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { return name === element.name; }); } + + deleteButtonClicked(header: ColumnHeader) { + this.removeColumn.emit({ + headerType: header.type, + }); + } } diff --git a/tensorboard/webapp/widgets/data_table/data_table_test.ts b/tensorboard/webapp/widgets/data_table/data_table_test.ts index 2a64f3a83b..0a55f9f3f9 100644 --- a/tensorboard/webapp/widgets/data_table/data_table_test.ts +++ b/tensorboard/webapp/widgets/data_table/data_table_test.ts @@ -52,6 +52,7 @@ import {HeaderCellComponent} from './header_cell_component'; " [header]="header" [sortingInfo]="sortingInfo" + [hparamsEnabled]="hparamsEnabled" > @@ -89,6 +90,7 @@ describe('data table', () => { data?: TableData[]; sortingInfo?: SortingInfo; smoothingEnabled?: boolean; + hparamsEnabled?: boolean; }): ComponentFixture { const fixture = TestBed.createComponent(TestableComponent); @@ -517,32 +519,32 @@ describe('data table', () => { expect( headerElements[0] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(true); expect( headerElements[0] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.getAttribute('svgIcon') ).toBe('arrow_upward_24px'); expect( headerElements[1] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(false); expect( headerElements[1] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show-on-hover') ).toBe(true); expect( headerElements[2] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(false); expect( headerElements[2] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show-on-hover') ).toBe(true); }); @@ -581,32 +583,32 @@ describe('data table', () => { expect( headerElements[0] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(false); expect( headerElements[0] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show-on-hover') ).toBe(true); expect( headerElements[1] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(false); expect( headerElements[1] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show-on-hover') ).toBe(true); expect( headerElements[2] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.classList.contains('show') ).toBe(true); expect( headerElements[2] - .queryAll(By.css('mat-icon'))[0] + .query(By.css('.sorting-icon-container mat-icon')) .nativeElement.getAttribute('svgIcon') ).toBe('arrow_downward_24px'); }); diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html b/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html index 5005491c7b..9933fa06d1 100644 --- a/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html +++ b/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html @@ -22,6 +22,14 @@ [ngClass]="highlightStyle$ | async" > +
+ + +
(); @Output() dragEnd = new EventEmitter(); @Output() dragEnter = new EventEmitter(); @Output() headerClicked = new EventEmitter(); + @Output() deleteButtonClicked = new EventEmitter<{ + headerType: ColumnHeaderType; + }>(); highlightStyle$: BehaviorSubject = new BehaviorSubject({}); diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts b/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts index 73f2ccb3a6..7e8273134e 100644 --- a/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts +++ b/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts @@ -34,7 +34,9 @@ import {HeaderCellComponent} from './header_cell_component'; `, }) @@ -44,12 +46,15 @@ class TestableComponent { @Input() header!: ColumnHeader; @Input() sortingInfo!: SortingInfo; + @Input() hparamsEnabled?: boolean; @Input() headerClicked!: (sortingInfo: SortingInfo) => void; + @Input() deleteButtonClicked!: (header: ColumnHeader) => void; } describe('header cell', () => { let headerClickedSpy: jasmine.Spy; + let deleteButtonClickedSpy: jasmine.Spy; beforeEach(async () => { await TestBed.configureTestingModule({ declarations: [TestableComponent, HeaderCellComponent], @@ -77,6 +82,9 @@ describe('header cell', () => { headerClickedSpy = jasmine.createSpy(); fixture.componentInstance.headerClicked = headerClickedSpy; + deleteButtonClickedSpy = jasmine.createSpy(); + fixture.componentInstance.deleteButtonClicked = deleteButtonClickedSpy; + return fixture; } @@ -96,4 +104,40 @@ describe('header cell', () => { fixture.debugElement.query(By.css('.cell')).nativeElement.click(); expect(headerClickedSpy).toHaveBeenCalledOnceWith('run'); }); + + describe('delete column button', () => { + let fixture: ComponentFixture; + beforeEach(() => { + fixture = createComponent({}); + fixture.componentInstance.hparamsEnabled = true; + fixture.detectChanges(); + }); + + it('emits removeColumn event when delete button clicked', () => { + fixture.debugElement + .query(By.directive(HeaderCellComponent)) + .componentInstance.deleteButtonClicked.subscribe(); + fixture.debugElement + .query(By.css('.delete-icon')) + .triggerEventHandler('click', {}); + + expect(deleteButtonClickedSpy).toHaveBeenCalledOnceWith({ + name: 'run', + displayName: 'Run', + type: ColumnHeaderType.RUN, + enabled: true, + }); + }); + + it('renders delete button when hparamsEnabled is true', () => { + expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeTruthy(); + }); + + it('does not render delete button when hparamsEnabled is false', () => { + fixture.componentInstance.hparamsEnabled = false; + fixture.detectChanges(); + + expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeFalsy(); + }); + }); });