From 1a2937ceac53b3ba2f36d264a6201f3d6d310e1e Mon Sep 17 00:00:00 2001 From: japie Date: Sat, 18 Mar 2023 13:14:01 -0700 Subject: [PATCH] Shareability: use table expansion from state for fullsize mode (#6243) To be able to remember the fullsize mode settings, we need to have it in state and also take the value from state as the source of truth for fullsize mode. --- tensorboard/webapp/metrics/actions/index.ts | 5 + .../webapp/metrics/store/metrics_reducers.ts | 13 ++ .../metrics/store/metrics_reducers_test.ts | 51 ++++++++ .../webapp/metrics/store/metrics_types.ts | 1 + .../webapp/metrics/views/card_renderer/BUILD | 1 + .../card_renderer/card_view_component.ng.html | 4 - .../views/card_renderer/card_view_test.ts | 32 +---- .../histogram_card_component.ng.html | 2 +- .../card_renderer/histogram_card_component.ts | 2 +- .../card_renderer/histogram_card_container.ts | 19 +-- .../card_renderer/histogram_card_test.ts | 30 ++--- .../scalar_card_component.ng.html | 2 +- .../card_renderer/scalar_card_component.ts | 2 +- .../card_renderer/scalar_card_container.ts | 21 +--- .../views/card_renderer/scalar_card_test.ts | 18 +-- .../main_view/card_grid_component.ng.html | 4 +- .../views/main_view/card_grid_component.ts | 2 + .../views/main_view/card_grid_container.ts | 3 + .../metrics/views/main_view/card_grid_test.ts | 116 ++++++++++++------ 19 files changed, 204 insertions(+), 124 deletions(-) diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index 05c34f1d15..0622518dc0 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -72,6 +72,11 @@ export const metricsCardStateUpdated = createAction( }>() ); +export const metricsCardFullSizeToggled = createAction( + '[Metrics] Metrics Card Full Size Toggled', + props<{cardId: CardId}>() +); + export const metricsChangeTooltipSort = createAction( '[Metrics] Metrics Settings Change Tooltip', props<{sort: TooltipSort}>() diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 831ff92302..ff030cb7a3 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -619,6 +619,19 @@ const reducer = createReducer( cardStateMap: nextcardStateMap, }; }), + on(actions.metricsCardFullSizeToggled, (state, {cardId}) => { + const nextcardStateMap = {...state.cardStateMap}; + nextcardStateMap[cardId] = { + ...nextcardStateMap[cardId], + fullWidth: !nextcardStateMap[cardId]?.fullWidth, + tableExpanded: !nextcardStateMap[cardId]?.fullWidth, + }; + + return { + ...state, + cardStateMap: nextcardStateMap, + }; + }), on(actions.metricsTagFilterChanged, (state, {tagFilter}) => { return { ...state, diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index ac5ec672cb..1ccdfccdd3 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2336,6 +2336,57 @@ describe('metrics reducers', () => { }); }); + describe('metricsCardFullSizeToggled', () => { + it('expands card', () => { + const state = buildMetricsState(); + const action = actions.metricsCardFullSizeToggled({ + cardId: 'card1', + }); + const nextState = reducers(state, action); + expect(nextState.cardStateMap).toEqual({ + card1: {fullWidth: true, tableExpanded: true}, + }); + }); + + it('expands card when table is already expanded', () => { + const state = buildMetricsState({ + cardStateMap: {card1: {tableExpanded: true}}, + }); + const action = actions.metricsCardFullSizeToggled({ + cardId: 'card1', + }); + const nextState = reducers(state, action); + expect(nextState.cardStateMap).toEqual({ + card1: {fullWidth: true, tableExpanded: true}, + }); + }); + + it('collapse card', () => { + const state = buildMetricsState({ + cardStateMap: {card1: {fullWidth: true}}, + }); + const action = actions.metricsCardFullSizeToggled({ + cardId: 'card1', + }); + const nextState = reducers(state, action); + expect(nextState.cardStateMap).toEqual({ + card1: {fullWidth: false, tableExpanded: false}, + }); + }); + + it('collapses card when table is already expanded', () => { + const state = buildMetricsState({ + cardStateMap: {card1: {fullWidth: true, tableExpanded: true}}, + }); + const action = actions.metricsCardFullSizeToggled({ + cardId: 'card1', + }); + const nextState = reducers(state, action); + expect(nextState.cardStateMap).toEqual({ + card1: {fullWidth: false, tableExpanded: false}, + }); + }); + }); describe('metricsTagFilterChanged', () => { it('sets the tagFilter state', () => { const state = buildMetricsState({tagFilter: 'foo'}); diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index 8753aded84..528e85eb14 100644 --- a/tensorboard/webapp/metrics/store/metrics_types.ts +++ b/tensorboard/webapp/metrics/store/metrics_types.ts @@ -139,6 +139,7 @@ export type CardState = { stepSelectionOverride: CardFeatureOverride; rangeSelectionOverride: CardFeatureOverride; tableExpanded: boolean; + fullWidth: boolean; }; export type CardStateMap = Record>; diff --git a/tensorboard/webapp/metrics/views/card_renderer/BUILD b/tensorboard/webapp/metrics/views/card_renderer/BUILD index dcfdd3ee40..598a819e18 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/BUILD +++ b/tensorboard/webapp/metrics/views/card_renderer/BUILD @@ -104,6 +104,7 @@ tf_ng_module( ":utils", ":vis_linked_time_selection_warning", "//tensorboard/webapp:app_state", + "//tensorboard/webapp:selectors", "//tensorboard/webapp/angular:expect_angular_material_button", "//tensorboard/webapp/angular:expect_angular_material_icon", "//tensorboard/webapp/angular:expect_angular_material_progress_spinner", diff --git a/tensorboard/webapp/metrics/views/card_renderer/card_view_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/card_view_component.ng.html index 68c5668432..3f02e5105d 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/card_view_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/card_view_component.ng.html @@ -29,8 +29,6 @@ *ngSwitchCase="PluginType.SCALARS" [cardId]="cardId" [groupName]="groupName" - (fullWidthChanged)="onFullWidthChanged($event)" - (fullHeightChanged)="onFullHeightChanged($event)" (pinStateChanged)="onPinStateChanged()" > @@ -39,8 +37,6 @@ [cardId]="cardId" [groupName]="groupName" [runColorScale]="runColorScale" - (fullWidthChanged)="onFullWidthChanged($event)" - (fullHeightChanged)="onFullHeightChanged($event)" (pinStateChanged)="onPinStateChanged()" > diff --git a/tensorboard/webapp/metrics/views/card_renderer/card_view_test.ts b/tensorboard/webapp/metrics/views/card_renderer/card_view_test.ts index 0ac031df03..48a29804b4 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/card_view_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/card_view_test.ts @@ -108,7 +108,7 @@ describe('card view test', () => { it('emits fullWidthChanged after lower level fullWidthChanged', () => { const fixture = TestBed.createComponent(CardViewContainer); fixture.componentInstance.cardId = 'cardId'; - fixture.componentInstance.pluginType = PluginType.SCALARS; + fixture.componentInstance.pluginType = PluginType.IMAGES; intersectionObserver.simulateVisibilityChange(fixture, true); fixture.detectChanges(); @@ -117,42 +117,18 @@ describe('card view test', () => { expect(onFullWidthChanged.calls.allArgs()).toEqual([]); - const scalarCard = fixture.debugElement.query(By.css('scalar-card')); - scalarCard.componentInstance.fullWidthChanged.emit(true); + const imageCard = fixture.debugElement.query(By.css('image-card')); + imageCard.componentInstance.fullWidthChanged.emit(true); fixture.detectChanges(); expect(onFullWidthChanged.calls.allArgs()).toEqual([[true]]); - scalarCard.componentInstance.fullWidthChanged.emit(false); + imageCard.componentInstance.fullWidthChanged.emit(false); fixture.detectChanges(); expect(onFullWidthChanged.calls.allArgs()).toEqual([[true], [false]]); }); - it('emits fullHeightChanged after lower level fullHeightChanged', () => { - const fixture = TestBed.createComponent(CardViewContainer); - fixture.componentInstance.cardId = 'cardId'; - fixture.componentInstance.pluginType = PluginType.SCALARS; - intersectionObserver.simulateVisibilityChange(fixture, true); - fixture.detectChanges(); - - const onFullHeightChanged = jasmine.createSpy(); - fixture.componentInstance.fullHeightChanged.subscribe(onFullHeightChanged); - - expect(onFullHeightChanged.calls.allArgs()).toEqual([]); - - const scalarCard = fixture.debugElement.query(By.css('scalar-card')); - scalarCard.componentInstance.fullHeightChanged.emit(true); - fixture.detectChanges(); - - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true]]); - - scalarCard.componentInstance.fullHeightChanged.emit(false); - fixture.detectChanges(); - - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true], [false]]); - }); - it('dispatches action when pin state changes', () => { const fixture = TestBed.createComponent(CardViewContainer); fixture.componentInstance.cardId = 'cardId'; diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ng.html index 2eef38655b..78e3e143cc 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ng.html @@ -50,7 +50,7 @@ (click)="onFullSizeToggle.emit()" > diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts index e3d4b503f3..1cf04ffb06 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts @@ -47,7 +47,7 @@ export class HistogramCardComponent { @Input() mode!: HistogramMode; @Input() xAxisType!: XAxisType; @Input() runColorScale!: RunColorScale; - @Input() showFullSize!: boolean; + @Input() showFullWidth!: boolean; @Input() isPinned!: boolean; @Input() linkedTimeSelection!: TimeSelectionView | null; @Input() isClosestStepHighlighted!: boolean | null; diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts index 7a20e27e7d..834d1957ec 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts @@ -32,7 +32,11 @@ import { } from '../../../widgets/card_fob/card_fob_types'; import {HistogramDatum} from '../../../widgets/histogram/histogram_types'; import {buildNormalizedHistograms} from '../../../widgets/histogram/histogram_util'; -import {stepSelectorToggled, timeSelectionChanged} from '../../actions'; +import { + metricsCardFullSizeToggled, + stepSelectorToggled, + timeSelectionChanged, +} from '../../actions'; import {HistogramStepDatum, PluginType} from '../../data_source'; import { getCardLoadState, @@ -43,6 +47,7 @@ import { getMetricsLinkedTimeSelection, getMetricsXAxisType, } from '../../store'; +import {getCardStateMap} from '../../../selectors'; import {CardId, CardMetadata} from '../../types'; import {CardRenderer} from '../metrics_view_types'; import {getTagDisplayName} from '../utils'; @@ -69,7 +74,7 @@ type HistogramCardMetadata = CardMetadata & { [mode]="mode$ | async" [xAxisType]="xAxisType$ | async" [runColorScale]="runColorScale" - [showFullSize]="showFullSize" + [showFullWidth]="showFullWidth$ | async" [isPinned]="isPinned$ | async" [isClosestStepHighlighted]="isClosestStepHighlighted$ | async" [linkedTimeSelection]="linkedTimeSelection$ | async" @@ -96,8 +101,6 @@ export class HistogramCardContainer implements CardRenderer, OnInit { @Input() cardId!: CardId; @Input() groupName!: string | null; @Input() runColorScale!: RunColorScale; - @Output() fullWidthChanged = new EventEmitter(); - @Output() fullHeightChanged = new EventEmitter(); @Output() pinStateChanged = new EventEmitter(); loadState$?: Observable; @@ -107,7 +110,9 @@ export class HistogramCardContainer implements CardRenderer, OnInit { data$?: Observable; mode$ = this.store.select(getMetricsHistogramMode); xAxisType$ = this.store.select(getMetricsXAxisType); - showFullSize = false; + readonly showFullWidth$ = this.store + .select(getCardStateMap) + .pipe(map((map) => map[this.cardId]?.fullWidth)); isPinned$?: Observable; linkedTimeSelection$?: Observable; isClosestStepHighlighted$?: Observable; @@ -122,9 +127,7 @@ export class HistogramCardContainer implements CardRenderer, OnInit { } onFullSizeToggle() { - this.showFullSize = !this.showFullSize; - this.fullWidthChanged.emit(this.showFullSize); - this.fullHeightChanged.emit(this.showFullSize); + this.store.dispatch(metricsCardFullSizeToggled({cardId: this.cardId})); } /** diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts index 9baa14b0de..0810f3b1d2 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts @@ -38,7 +38,11 @@ import { } from '../../../widgets/histogram/histogram_types'; import {buildNormalizedHistograms} from '../../../widgets/histogram/histogram_util'; import {TruncatedPathModule} from '../../../widgets/text/truncated_path_module'; -import {stepSelectorToggled, timeSelectionChanged} from '../../actions'; +import { + stepSelectorToggled, + timeSelectionChanged, + metricsCardFullSizeToggled, +} from '../../actions'; import {PluginType} from '../../data_source'; import * as selectors from '../../store/metrics_selectors'; import { @@ -256,31 +260,29 @@ describe('histogram card', () => { }); describe('full size', () => { + let dispatchedActions: Action[]; + beforeEach(() => { provideMockCardSeriesData(selectSpy, PluginType.HISTOGRAMS, 'card1'); + + dispatchedActions = []; + spyOn(store, 'dispatch').and.callFake((action: Action) => { + dispatchedActions.push(action); + }); }); - it('requests full size on toggle', () => { - const onFullWidthChanged = jasmine.createSpy(); - const onFullHeightChanged = jasmine.createSpy(); + it('dispatches metricsCardFullSizeToggled on full size toggle', () => { const fixture = createHistogramCardContainer(); fixture.detectChanges(); - fixture.componentInstance.fullWidthChanged.subscribe(onFullWidthChanged); - fixture.componentInstance.fullHeightChanged.subscribe( - onFullHeightChanged - ); const button = fixture.debugElement.query( By.css('[aria-label="Toggle full size mode"]') ); button.nativeElement.click(); - expect(onFullWidthChanged.calls.allArgs()).toEqual([[true]]); - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true]]); - - button.nativeElement.click(); - expect(onFullWidthChanged.calls.allArgs()).toEqual([[true], [false]]); - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true], [false]]); + expect(dispatchedActions).toEqual([ + metricsCardFullSizeToggled({cardId: 'card1'}), + ]); }); }); 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 f462115e59..f04dbd92ff 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 @@ -62,7 +62,7 @@ (click)="onFullSizeToggle.emit()" > 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 b91e5aaf55..e36dc44b64 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -83,7 +83,7 @@ export class ScalarCardComponent { @Input() isCardVisible!: boolean; @Input() isPinned!: boolean; @Input() loadState!: DataLoadState; - @Input() showFullSize!: boolean; + @Input() showFullWidth!: boolean; @Input() smoothingEnabled!: boolean; @Input() tag!: string; @Input() title!: string; 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 36cd191758..62121fc3de 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts @@ -76,6 +76,7 @@ import { cardMinMaxChanged, dataTableColumnDrag, metricsCardStateUpdated, + metricsCardFullSizeToggled, sortingDataTable, stepSelectorToggled, timeSelectionChanged, @@ -164,7 +165,7 @@ function isMinMaxStepValid(minMax: MinMaxStep | undefined): boolean { [isCardVisible]="isVisible" [isPinned]="isPinned$ | async" [loadState]="loadState$ | async" - [showFullSize]="showFullSize" + [showFullWidth]="showFullWidth$ | async" [smoothingEnabled]="smoothingEnabled$ | async" [tag]="tag$ | async" [title]="title$ | async" @@ -212,8 +213,6 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { DataDownloadDialogContainer; @Input() cardId!: CardId; @Input() groupName!: string | null; - @Output() fullWidthChanged = new EventEmitter(); - @Output() fullHeightChanged = new EventEmitter(); @Output() pinStateChanged = new EventEmitter(); isVisible: boolean = false; @@ -273,7 +272,9 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { .select(getMetricsScalarSmoothing) .pipe(map((smoothing) => smoothing > 0)); - showFullSize = false; + readonly showFullWidth$ = this.store + .select(getCardStateMap) + .pipe(map((map) => map[this.cardId]?.fullWidth)); private readonly ngUnsubscribe = new Subject(); @@ -285,17 +286,7 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy { } onFullSizeToggle() { - this.showFullSize = !this.showFullSize; - this.fullWidthChanged.emit(this.showFullSize); - this.fullHeightChanged.emit(this.showFullSize); - this.store.dispatch( - metricsCardStateUpdated({ - cardId: this.cardId, - settings: { - tableExpanded: this.showFullSize, - }, - }) - ); + this.store.dispatch(metricsCardFullSizeToggled({cardId: this.cardId})); } /** 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 c9aace946e..c37bbf7081 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -77,6 +77,7 @@ import {TruncatedPathModule} from '../../../widgets/text/truncated_path_module'; import { cardMinMaxChanged, metricsCardStateUpdated, + metricsCardFullSizeToggled, stepSelectorToggled, timeSelectionChanged, } from '../../actions'; @@ -850,27 +851,18 @@ describe('scalar card', () => { ); }); - it('requests full size on toggle', fakeAsync(() => { - const onFullWidthChanged = jasmine.createSpy(); - const onFullHeightChanged = jasmine.createSpy(); + it('dispatches metricsCardFullSizeToggled on full size toggle', fakeAsync(() => { const fixture = createComponent('card1'); fixture.detectChanges(); - fixture.componentInstance.fullWidthChanged.subscribe(onFullWidthChanged); - fixture.componentInstance.fullHeightChanged.subscribe( - onFullHeightChanged - ); const button = fixture.debugElement.query( By.css('[aria-label="Toggle full size mode"]') ); button.nativeElement.click(); - expect(onFullWidthChanged.calls.allArgs()).toEqual([[true]]); - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true]]); - - button.nativeElement.click(); - expect(onFullWidthChanged.calls.allArgs()).toEqual([[true], [false]]); - expect(onFullHeightChanged.calls.allArgs()).toEqual([[true], [false]]); + expect(dispatchedActions).toEqual([ + metricsCardFullSizeToggled({cardId: 'card1'}), + ]); })); }); diff --git a/tensorboard/webapp/metrics/views/main_view/card_grid_component.ng.html b/tensorboard/webapp/metrics/views/main_view/card_grid_component.ng.html index 8f80b7c0e1..9df3660a0d 100644 --- a/tensorboard/webapp/metrics/views/main_view/card_grid_component.ng.html +++ b/tensorboard/webapp/metrics/views/main_view/card_grid_component.ng.html @@ -25,8 +25,8 @@ *ngFor="let item of cardIdsWithMetadata; trackBy: trackByCards" class="card-space" [ngClass]="{ - 'full-width': cardsAtFullWidth.has(item.cardId), - 'full-height': cardsAtFullHeight.has(item.cardId) + 'full-width': cardsAtFullWidth.has(item.cardId) || cardStateMap[item.cardId]?.fullWidth, + 'full-height': cardsAtFullHeight.has(item.cardId) || cardStateMap[item.cardId]?.tableExpanded }" > (); diff --git a/tensorboard/webapp/metrics/views/main_view/card_grid_container.ts b/tensorboard/webapp/metrics/views/main_view/card_grid_container.ts index 55f9e43876..9c4d4e04d5 100644 --- a/tensorboard/webapp/metrics/views/main_view/card_grid_container.ts +++ b/tensorboard/webapp/metrics/views/main_view/card_grid_container.ts @@ -31,6 +31,7 @@ import { import {selectors as settingsSelectors} from '../../../settings'; import {CardObserver} from '../card_renderer/card_lazy_loader'; import {CardIdWithMetadata} from '../metrics_view_types'; +import * as selectors from '../../../selectors'; @Component({ selector: 'metrics-card-grid', @@ -43,6 +44,7 @@ import {CardIdWithMetadata} from '../metrics_view_types'; [cardIdsWithMetadata]="pagedItems$ | async" [cardMinWidth]="cardMinWidth$ | async" [cardObserver]="cardObserver" + [cardStateMap]="cardStateMap$ | async" (pageIndexChanged)="onPageIndexChanged($event)" > @@ -59,6 +61,7 @@ export class CardGridContainer implements OnChanges, OnDestroy { readonly pageIndex$ = new BehaviorSubject(0); private readonly items$ = new BehaviorSubject([]); private readonly ngUnsubscribe = new Subject(); + readonly cardStateMap$ = this.store.select(selectors.getCardStateMap); readonly numPages$ = combineLatest([ this.items$, diff --git a/tensorboard/webapp/metrics/views/main_view/card_grid_test.ts b/tensorboard/webapp/metrics/views/main_view/card_grid_test.ts index 897d11f1d5..a9488e25fd 100644 --- a/tensorboard/webapp/metrics/views/main_view/card_grid_test.ts +++ b/tensorboard/webapp/metrics/views/main_view/card_grid_test.ts @@ -34,6 +34,7 @@ import {MockStore} from '@ngrx/store/testing'; import {State} from '../../../app_state'; import * as selectors from '../../../selectors'; import { + getCardStateMap, getMetricsCardMinWidth, getMetricsTagGroupExpansionState, } from '../../../selectors'; @@ -106,6 +107,7 @@ describe('card grid', () => { store.overrideSelector(getMetricsTagGroupExpansionState, true); store.overrideSelector(getMetricsCardMinWidth, 30); store.overrideSelector(settingsSelectors.getPageSize, 10); + store.overrideSelector(getCardStateMap, {}); }); afterEach(() => { @@ -230,7 +232,8 @@ describe('card grid', () => { describe('card dimensions', () => { let fixture: ComponentFixture; - beforeEach(() => { + + function createComponent() { fixture = TestBed.createComponent(TestableScrollingContainer); fixture.componentInstance.cardIdsWithMetadata = [ { @@ -253,9 +256,12 @@ describe('card grid', () => { }, ]; fixture.detectChanges(); - }); + + return fixture; + } it('shows cards at min dimensions by default', () => { + const fixture = createComponent(); const cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); expect(cardSpaces[0].nativeElement.classList).not.toContain( 'full-height' @@ -272,6 +278,7 @@ describe('card grid', () => { }); it('changes height after card event', () => { + const fixture = createComponent(); const cardViews = fixture.debugElement.queryAll(By.css('card-view')); const cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); @@ -317,6 +324,7 @@ describe('card grid', () => { }); it('does not change height if emitted value is same', () => { + const fixture = createComponent(); const cardViews = fixture.debugElement.queryAll(By.css('card-view')); const cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); @@ -351,56 +359,92 @@ describe('card grid', () => { ); }); - it('changes width after card event', () => { - const cardViews = fixture.debugElement.queryAll(By.css('card-view')); - const cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); + it('renders card width based on card state table expanded', () => { + store.overrideSelector(getCardStateMap, {card2: {tableExpanded: true}}); + let fixture = createComponent(); + let cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); + expect(cardSpaces[0].nativeElement.classList).not.toContain( + 'full-height' + ); + expect(cardSpaces[1].nativeElement.classList).toContain('full-height'); + expect(cardSpaces[2].nativeElement.classList).not.toContain( + 'full-height' + ); - cardViews[2].componentInstance.fullWidthChanged.emit(true); - fixture.detectChanges(); + store.overrideSelector(getCardStateMap, { + card1: {tableExpanded: true}, + card2: {tableExpanded: true}, + }); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); + expect(cardSpaces[0].nativeElement.classList).toContain('full-height'); + expect(cardSpaces[1].nativeElement.classList).toContain('full-height'); + expect(cardSpaces[2].nativeElement.classList).not.toContain( + 'full-height' + ); + + store.overrideSelector(getCardStateMap, { + card1: {tableExpanded: false}, + card2: {tableExpanded: true}, + }); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); + expect(cardSpaces[0].nativeElement.classList).not.toContain( + 'full-height' + ); + expect(cardSpaces[1].nativeElement.classList).toContain('full-height'); + expect(cardSpaces[2].nativeElement.classList).not.toContain( + 'full-height' + ); + + store.overrideSelector(getCardStateMap, {}); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); + expect(cardSpaces[0].nativeElement.classList).not.toContain( + 'full-height' + ); + expect(cardSpaces[1].nativeElement.classList).not.toContain( + 'full-height' + ); + expect(cardSpaces[2].nativeElement.classList).not.toContain( + 'full-height' + ); + }); + + it('renders card width based on card state full width', () => { + store.overrideSelector(getCardStateMap, {card3: {fullWidth: true}}); + let fixture = createComponent(); + let cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[1].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[2].nativeElement.classList).toContain('full-width'); - cardViews[1].componentInstance.fullWidthChanged.emit(true); - fixture.detectChanges(); + store.overrideSelector(getCardStateMap, { + card2: {fullWidth: true}, + card3: {fullWidth: true}, + }); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[1].nativeElement.classList).toContain('full-width'); expect(cardSpaces[2].nativeElement.classList).toContain('full-width'); - cardViews[1].componentInstance.fullWidthChanged.emit(false); - fixture.detectChanges(); + store.overrideSelector(getCardStateMap, { + card2: {fullWidth: false}, + card3: {fullWidth: true}, + }); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[1].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[2].nativeElement.classList).toContain('full-width'); - cardViews[2].componentInstance.fullWidthChanged.emit(false); - fixture.detectChanges(); + store.overrideSelector(getCardStateMap, {}); + fixture = createComponent(); + cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[1].nativeElement.classList).not.toContain('full-width'); expect(cardSpaces[2].nativeElement.classList).not.toContain('full-width'); }); - - it('does not change width if emitted value is same', () => { - const cardViews = fixture.debugElement.queryAll(By.css('card-view')); - const cardSpaces = fixture.debugElement.queryAll(By.css('.card-space')); - - cardViews[1].componentInstance.fullWidthChanged.emit(true); - fixture.detectChanges(); - expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); - expect(cardSpaces[1].nativeElement.classList).toContain('full-width'); - expect(cardSpaces[2].nativeElement.classList).not.toContain('full-width'); - - cardViews[0].componentInstance.fullWidthChanged.emit(false); - fixture.detectChanges(); - expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); - expect(cardSpaces[1].nativeElement.classList).toContain('full-width'); - expect(cardSpaces[2].nativeElement.classList).not.toContain('full-width'); - - cardViews[1].componentInstance.fullWidthChanged.emit(true); - fixture.detectChanges(); - expect(cardSpaces[0].nativeElement.classList).not.toContain('full-width'); - expect(cardSpaces[1].nativeElement.classList).toContain('full-width'); - expect(cardSpaces[2].nativeElement.classList).not.toContain('full-width'); - }); }); });