Shareability: use table expansion from state for fullsize mode (tenso…

…rflow#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

@@ -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}>()

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,

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'});

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<CardId, Partial<CardState>>;

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",

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()"
   ></scalar-card>
 
@@ -39,8 +37,6 @@
     [cardId]="cardId"
     [groupName]="groupName"
     [runColorScale]="runColorScale"
-    (fullWidthChanged)="onFullWidthChanged($event)"
-    (fullHeightChanged)="onFullHeightChanged($event)"
     (pinStateChanged)="onPinStateChanged()"
   ></histogram-card>
 

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';

tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ng.html

@@ -50,7 +50,7 @@
       (click)="onFullSizeToggle.emit()"
     >
       <mat-icon
-        [svgIcon]="showFullSize ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
+        [svgIcon]="showFullWidth ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
       ></mat-icon>
     </button>
   </span>

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;

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<boolean>();
-  @Output() fullHeightChanged = new EventEmitter<boolean>();
   @Output() pinStateChanged = new EventEmitter<boolean>();
 
   loadState$?: Observable<DataLoadState>;
@@ -107,7 +110,9 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
   data$?: Observable<HistogramDatum[]>;
   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<boolean>;
   linkedTimeSelection$?: Observable<TimeSelectionView | null>;
   isClosestStepHighlighted$?: Observable<boolean | null>;
@@ -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}));
   }
 
   /**

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'}),
+      ]);
     });
   });
 

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

@@ -62,7 +62,7 @@
         (click)="onFullSizeToggle.emit()"
       >
         <mat-icon
-          [svgIcon]="showFullSize ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
+          [svgIcon]="showFullWidth ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
         ></mat-icon>
       </button>
       <!-- overflow menu cannot use mat-tooltip since it interferes with the mat-menu. -->

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts

@@ -83,7 +83,7 @@ export class ScalarCardComponent<Downloader> {
   @Input() isCardVisible!: boolean;
   @Input() isPinned!: boolean;
   @Input() loadState!: DataLoadState;
-  @Input() showFullSize!: boolean;
+  @Input() showFullWidth!: boolean;
   @Input() smoothingEnabled!: boolean;
   @Input() tag!: string;
   @Input() title!: string;

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<boolean>();
-  @Output() fullHeightChanged = new EventEmitter<boolean>();
   @Output() pinStateChanged = new EventEmitter<boolean>();
 
   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<void>();
 
@@ -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}));
   }
 
   /**