Skip to content

Commit 1a2937c

Browse files
japie1235813japie1235813
authored
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.
1 parent a30f5dc commit 1a2937c

19 files changed

+204
-124
lines changed

‎tensorboard/webapp/metrics/actions/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ export const metricsCardStateUpdated = createAction(
7272
}>()
7373
);
7474

75+
export const metricsCardFullSizeToggled = createAction(
76+
'[Metrics] Metrics Card Full Size Toggled',
77+
props<{cardId: CardId}>()
78+
);
79+
7580
export const metricsChangeTooltipSort = createAction(
7681
'[Metrics] Metrics Settings Change Tooltip',
7782
props<{sort: TooltipSort}>()

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,19 @@ const reducer = createReducer(
619619
cardStateMap: nextcardStateMap,
620620
};
621621
}),
622+
on(actions.metricsCardFullSizeToggled, (state, {cardId}) => {
623+
const nextcardStateMap = {...state.cardStateMap};
624+
nextcardStateMap[cardId] = {
625+
...nextcardStateMap[cardId],
626+
fullWidth: !nextcardStateMap[cardId]?.fullWidth,
627+
tableExpanded: !nextcardStateMap[cardId]?.fullWidth,
628+
};
629+
630+
return {
631+
...state,
632+
cardStateMap: nextcardStateMap,
633+
};
634+
}),
622635
on(actions.metricsTagFilterChanged, (state, {tagFilter}) => {
623636
return {
624637
...state,

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2336,6 +2336,57 @@ describe('metrics reducers', () => {
23362336
});
23372337
});
23382338

2339+
describe('metricsCardFullSizeToggled', () => {
2340+
it('expands card', () => {
2341+
const state = buildMetricsState();
2342+
const action = actions.metricsCardFullSizeToggled({
2343+
cardId: 'card1',
2344+
});
2345+
const nextState = reducers(state, action);
2346+
expect(nextState.cardStateMap).toEqual({
2347+
card1: {fullWidth: true, tableExpanded: true},
2348+
});
2349+
});
2350+
2351+
it('expands card when table is already expanded', () => {
2352+
const state = buildMetricsState({
2353+
cardStateMap: {card1: {tableExpanded: true}},
2354+
});
2355+
const action = actions.metricsCardFullSizeToggled({
2356+
cardId: 'card1',
2357+
});
2358+
const nextState = reducers(state, action);
2359+
expect(nextState.cardStateMap).toEqual({
2360+
card1: {fullWidth: true, tableExpanded: true},
2361+
});
2362+
});
2363+
2364+
it('collapse card', () => {
2365+
const state = buildMetricsState({
2366+
cardStateMap: {card1: {fullWidth: true}},
2367+
});
2368+
const action = actions.metricsCardFullSizeToggled({
2369+
cardId: 'card1',
2370+
});
2371+
const nextState = reducers(state, action);
2372+
expect(nextState.cardStateMap).toEqual({
2373+
card1: {fullWidth: false, tableExpanded: false},
2374+
});
2375+
});
2376+
2377+
it('collapses card when table is already expanded', () => {
2378+
const state = buildMetricsState({
2379+
cardStateMap: {card1: {fullWidth: true, tableExpanded: true}},
2380+
});
2381+
const action = actions.metricsCardFullSizeToggled({
2382+
cardId: 'card1',
2383+
});
2384+
const nextState = reducers(state, action);
2385+
expect(nextState.cardStateMap).toEqual({
2386+
card1: {fullWidth: false, tableExpanded: false},
2387+
});
2388+
});
2389+
});
23392390
describe('metricsTagFilterChanged', () => {
23402391
it('sets the tagFilter state', () => {
23412392
const state = buildMetricsState({tagFilter: 'foo'});

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ export type CardState = {
139139
stepSelectionOverride: CardFeatureOverride;
140140
rangeSelectionOverride: CardFeatureOverride;
141141
tableExpanded: boolean;
142+
fullWidth: boolean;
142143
};
143144

144145
export type CardStateMap = Record<CardId, Partial<CardState>>;

‎tensorboard/webapp/metrics/views/card_renderer/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ tf_ng_module(
104104
":utils",
105105
":vis_linked_time_selection_warning",
106106
"//tensorboard/webapp:app_state",
107+
"//tensorboard/webapp:selectors",
107108
"//tensorboard/webapp/angular:expect_angular_material_button",
108109
"//tensorboard/webapp/angular:expect_angular_material_icon",
109110
"//tensorboard/webapp/angular:expect_angular_material_progress_spinner",

‎tensorboard/webapp/metrics/views/card_renderer/card_view_component.ng.html

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
*ngSwitchCase="PluginType.SCALARS"
3030
[cardId]="cardId"
3131
[groupName]="groupName"
32-
(fullWidthChanged)="onFullWidthChanged($event)"
33-
(fullHeightChanged)="onFullHeightChanged($event)"
3432
(pinStateChanged)="onPinStateChanged()"
3533
></scalar-card>
3634

@@ -39,8 +37,6 @@
3937
[cardId]="cardId"
4038
[groupName]="groupName"
4139
[runColorScale]="runColorScale"
42-
(fullWidthChanged)="onFullWidthChanged($event)"
43-
(fullHeightChanged)="onFullHeightChanged($event)"
4440
(pinStateChanged)="onPinStateChanged()"
4541
></histogram-card>
4642

‎tensorboard/webapp/metrics/views/card_renderer/card_view_test.ts

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ describe('card view test', () => {
108108
it('emits fullWidthChanged after lower level fullWidthChanged', () => {
109109
const fixture = TestBed.createComponent(CardViewContainer);
110110
fixture.componentInstance.cardId = 'cardId';
111-
fixture.componentInstance.pluginType = PluginType.SCALARS;
111+
fixture.componentInstance.pluginType = PluginType.IMAGES;
112112
intersectionObserver.simulateVisibilityChange(fixture, true);
113113
fixture.detectChanges();
114114

@@ -117,42 +117,18 @@ describe('card view test', () => {
117117

118118
expect(onFullWidthChanged.calls.allArgs()).toEqual([]);
119119

120-
const scalarCard = fixture.debugElement.query(By.css('scalar-card'));
121-
scalarCard.componentInstance.fullWidthChanged.emit(true);
120+
const imageCard = fixture.debugElement.query(By.css('image-card'));
121+
imageCard.componentInstance.fullWidthChanged.emit(true);
122122
fixture.detectChanges();
123123

124124
expect(onFullWidthChanged.calls.allArgs()).toEqual([[true]]);
125125

126-
scalarCard.componentInstance.fullWidthChanged.emit(false);
126+
imageCard.componentInstance.fullWidthChanged.emit(false);
127127
fixture.detectChanges();
128128

129129
expect(onFullWidthChanged.calls.allArgs()).toEqual([[true], [false]]);
130130
});
131131

132-
it('emits fullHeightChanged after lower level fullHeightChanged', () => {
133-
const fixture = TestBed.createComponent(CardViewContainer);
134-
fixture.componentInstance.cardId = 'cardId';
135-
fixture.componentInstance.pluginType = PluginType.SCALARS;
136-
intersectionObserver.simulateVisibilityChange(fixture, true);
137-
fixture.detectChanges();
138-
139-
const onFullHeightChanged = jasmine.createSpy();
140-
fixture.componentInstance.fullHeightChanged.subscribe(onFullHeightChanged);
141-
142-
expect(onFullHeightChanged.calls.allArgs()).toEqual([]);
143-
144-
const scalarCard = fixture.debugElement.query(By.css('scalar-card'));
145-
scalarCard.componentInstance.fullHeightChanged.emit(true);
146-
fixture.detectChanges();
147-
148-
expect(onFullHeightChanged.calls.allArgs()).toEqual([[true]]);
149-
150-
scalarCard.componentInstance.fullHeightChanged.emit(false);
151-
fixture.detectChanges();
152-
153-
expect(onFullHeightChanged.calls.allArgs()).toEqual([[true], [false]]);
154-
});
155-
156132
it('dispatches action when pin state changes', () => {
157133
const fixture = TestBed.createComponent(CardViewContainer);
158134
fixture.componentInstance.cardId = 'cardId';

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
(click)="onFullSizeToggle.emit()"
5151
>
5252
<mat-icon
53-
[svgIcon]="showFullSize ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
53+
[svgIcon]="showFullWidth ? 'fullscreen_exit_24px' : 'fullscreen_24px'"
5454
></mat-icon>
5555
</button>
5656
</span>

‎tensorboard/webapp/metrics/views/card_renderer/histogram_card_component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class HistogramCardComponent {
4747
@Input() mode!: HistogramMode;
4848
@Input() xAxisType!: XAxisType;
4949
@Input() runColorScale!: RunColorScale;
50-
@Input() showFullSize!: boolean;
50+
@Input() showFullWidth!: boolean;
5151
@Input() isPinned!: boolean;
5252
@Input() linkedTimeSelection!: TimeSelectionView | null;
5353
@Input() isClosestStepHighlighted!: boolean | null;

‎tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ import {
3232
} from '../../../widgets/card_fob/card_fob_types';
3333
import {HistogramDatum} from '../../../widgets/histogram/histogram_types';
3434
import {buildNormalizedHistograms} from '../../../widgets/histogram/histogram_util';
35-
import {stepSelectorToggled, timeSelectionChanged} from '../../actions';
35+
import {
36+
metricsCardFullSizeToggled,
37+
stepSelectorToggled,
38+
timeSelectionChanged,
39+
} from '../../actions';
3640
import {HistogramStepDatum, PluginType} from '../../data_source';
3741
import {
3842
getCardLoadState,
@@ -43,6 +47,7 @@ import {
4347
getMetricsLinkedTimeSelection,
4448
getMetricsXAxisType,
4549
} from '../../store';
50+
import {getCardStateMap} from '../../../selectors';
4651
import {CardId, CardMetadata} from '../../types';
4752
import {CardRenderer} from '../metrics_view_types';
4853
import {getTagDisplayName} from '../utils';
@@ -69,7 +74,7 @@ type HistogramCardMetadata = CardMetadata & {
6974
[mode]="mode$ | async"
7075
[xAxisType]="xAxisType$ | async"
7176
[runColorScale]="runColorScale"
72-
[showFullSize]="showFullSize"
77+
[showFullWidth]="showFullWidth$ | async"
7378
[isPinned]="isPinned$ | async"
7479
[isClosestStepHighlighted]="isClosestStepHighlighted$ | async"
7580
[linkedTimeSelection]="linkedTimeSelection$ | async"
@@ -96,8 +101,6 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
96101
@Input() cardId!: CardId;
97102
@Input() groupName!: string | null;
98103
@Input() runColorScale!: RunColorScale;
99-
@Output() fullWidthChanged = new EventEmitter<boolean>();
100-
@Output() fullHeightChanged = new EventEmitter<boolean>();
101104
@Output() pinStateChanged = new EventEmitter<boolean>();
102105

103106
loadState$?: Observable<DataLoadState>;
@@ -107,7 +110,9 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
107110
data$?: Observable<HistogramDatum[]>;
108111
mode$ = this.store.select(getMetricsHistogramMode);
109112
xAxisType$ = this.store.select(getMetricsXAxisType);
110-
showFullSize = false;
113+
readonly showFullWidth$ = this.store
114+
.select(getCardStateMap)
115+
.pipe(map((map) => map[this.cardId]?.fullWidth));
111116
isPinned$?: Observable<boolean>;
112117
linkedTimeSelection$?: Observable<TimeSelectionView | null>;
113118
isClosestStepHighlighted$?: Observable<boolean | null>;
@@ -122,9 +127,7 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
122127
}
123128

124129
onFullSizeToggle() {
125-
this.showFullSize = !this.showFullSize;
126-
this.fullWidthChanged.emit(this.showFullSize);
127-
this.fullHeightChanged.emit(this.showFullSize);
130+
this.store.dispatch(metricsCardFullSizeToggled({cardId: this.cardId}));
128131
}
129132

130133
/**

0 commit comments

Comments
 (0)