Skip to content

Commit a9734f8

Browse files
authored
Column Customization: Open Editor to the expected tab (#6305)
## Motivation for features / changes When the Column Editor is opened from the scalar card it opens to whatever the default or last selected tab is. However, it would be expected to open to the mode that the card used to open it is in(Single vs Range). This makes that happen ## Technical description of changes This PR creates a new value in the metricsState(tableEditorSelectedTab) which reflects the selected tab in the table editor. It then pulls that value into the editor and uses it to select the tab. It is kept up to date with calls to tableEditorTabChanged (a new action created for this purpose) when the tab is changed inside the controller. Lastly it adds a new prop to the metricsSlideoutMenuOpened action which contains the current mode of the chart which dispatched the action. This mode is used to update the new tableEditorSelectedTab value. ## Screenshots of UI changes ![2023-04-13_23-47-03 (1)](https://user-images.githubusercontent.com/8672809/231965710-9b124054-cf47-4394-9789-18894e06574c.gif)
1 parent 8e852e5 commit a9734f8

16 files changed

+207
-21
lines changed

tensorboard/webapp/metrics/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ tf_ts_library(
8888
"//tensorboard/webapp/metrics/data_source",
8989
"//tensorboard/webapp/metrics/store",
9090
"//tensorboard/webapp/metrics/store:types",
91+
"//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types",
9192
"//tensorboard/webapp/types",
9293
"//tensorboard/webapp/widgets/histogram:types",
9394
"@npm//@angular/core",

tensorboard/webapp/metrics/actions/index.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
} from '../types';
3737
import {
3838
ColumnHeader,
39+
DataTableMode,
3940
SortingInfo,
4041
} from '../views/card_renderer/scalar_card_types';
4142

@@ -52,7 +53,13 @@ export const metricsSlideoutMenuToggled = createAction(
5253
);
5354

5455
export const metricsSlideoutMenuOpened = createAction(
55-
'[Metrics] User requested to open the slide out menu'
56+
'[Metrics] User requested to open the slide out menu',
57+
props<{mode: DataTableMode}>()
58+
);
59+
60+
export const tableEditorTabChanged = createAction(
61+
'[Metrics] User changed the tab in the table editor',
62+
props<{tab: DataTableMode}>()
5663
);
5764

5865
export const metricsSlideoutMenuClosed = createAction(

tensorboard/webapp/metrics/store/metrics_reducers.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ const {initialState, reducers: namespaceContextedReducer} =
299299
{
300300
isSettingsPaneOpen: true,
301301
isSlideoutMenuOpen: false,
302+
tableEditorSelectedTab: DataTableMode.SINGLE,
302303
timeSeriesData: {
303304
scalars: {},
304305
histograms: {},
@@ -1252,6 +1253,12 @@ const reducer = createReducer(
12521253
linkedTimeSelection: null,
12531254
};
12541255
}),
1256+
on(actions.tableEditorTabChanged, (state, {tab}) => {
1257+
return {
1258+
...state,
1259+
tableEditorSelectedTab: tab,
1260+
};
1261+
}),
12551262
on(actions.dataTableColumnEdited, (state, {dataTableMode, headers}) => {
12561263
const enabledNewHeaders: ColumnHeader[] = [];
12571264
const disabledNewHeaders: ColumnHeader[] = [];
@@ -1346,12 +1353,17 @@ const reducer = createReducer(
13461353
on(actions.metricsSlideoutMenuToggled, (state) => {
13471354
return {...state, isSlideoutMenuOpen: !state.isSlideoutMenuOpen};
13481355
}),
1349-
on(actions.metricsSlideoutMenuOpened, (state) => {
1356+
on(actions.metricsSlideoutMenuOpened, (state, {mode}) => {
13501357
// The reason the toggle action does not open the settings pane is because
13511358
// the settings pane is the only place the menu can be toggled. The open
13521359
// request can be made from the card when the settings menu is closed,
13531360
// therefore we need to make sure the settings menu is opened, too.
1354-
return {...state, isSlideoutMenuOpen: true, isSettingsPaneOpen: true};
1361+
return {
1362+
...state,
1363+
isSlideoutMenuOpen: true,
1364+
isSettingsPaneOpen: true,
1365+
tableEditorSelectedTab: mode,
1366+
};
13551367
}),
13561368
on(actions.metricsSlideoutMenuClosed, (state) => {
13571369
return {...state, isSlideoutMenuOpen: false};

tensorboard/webapp/metrics/store/metrics_reducers_test.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3929,19 +3929,27 @@ describe('metrics reducers', () => {
39293929
});
39303930

39313931
describe('#metricsSlideoutMenuOpened', () => {
3932-
it('sets the isSlideoutMenuOpen and isSettingsPaneOpen to true', () => {
3932+
it('sets the isSlideoutMenuOpen and isSettingsPaneOpen to true and always updates tableEditorSelectedTab', () => {
39333933
const state1 = buildMetricsState({
39343934
isSlideoutMenuOpen: false,
39353935
isSettingsPaneOpen: false,
39363936
});
39373937

3938-
const state2 = reducers(state1, actions.metricsSlideoutMenuOpened());
3938+
const state2 = reducers(
3939+
state1,
3940+
actions.metricsSlideoutMenuOpened({mode: DataTableMode.RANGE})
3941+
);
39393942
expect(state2.isSlideoutMenuOpen).toBe(true);
39403943
expect(state2.isSettingsPaneOpen).toBe(true);
3944+
expect(state2.tableEditorSelectedTab).toBe(DataTableMode.RANGE);
39413945

3942-
const state3 = reducers(state1, actions.metricsSlideoutMenuOpened());
3946+
const state3 = reducers(
3947+
state2,
3948+
actions.metricsSlideoutMenuOpened({mode: DataTableMode.SINGLE})
3949+
);
39433950
expect(state3.isSlideoutMenuOpen).toBe(true);
39443951
expect(state3.isSettingsPaneOpen).toBe(true);
3952+
expect(state3.tableEditorSelectedTab).toBe(DataTableMode.SINGLE);
39453953
});
39463954

39473955
it('leaves isSettingsPaneOpen as true when it is already set', () => {
@@ -3950,7 +3958,10 @@ describe('metrics reducers', () => {
39503958
isSettingsPaneOpen: true,
39513959
});
39523960

3953-
const state2 = reducers(state1, actions.metricsSlideoutMenuOpened());
3961+
const state2 = reducers(
3962+
state1,
3963+
actions.metricsSlideoutMenuOpened({mode: DataTableMode.SINGLE})
3964+
);
39543965
expect(state2.isSlideoutMenuOpen).toBe(true);
39553966
expect(state2.isSettingsPaneOpen).toBe(true);
39563967
});

tensorboard/webapp/metrics/store/metrics_selectors.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
} from '../types';
3232
import {
3333
ColumnHeader,
34+
DataTableMode,
3435
MinMaxStep,
3536
} from '../views/card_renderer/scalar_card_types';
3637
import {formatTimeSelection} from '../views/card_renderer/utils';
@@ -484,6 +485,11 @@ export const isMetricsSlideoutMenuOpen = createSelector(
484485
(state): boolean => state.isSlideoutMenuOpen
485486
);
486487

488+
export const getTableEditorSelectedTab = createSelector(
489+
selectMetricsState,
490+
(state): DataTableMode => state.tableEditorSelectedTab
491+
);
492+
487493
export const getMetricsCardRangeSelectionEnabled = createSelector(
488494
getMetricsRangeSelectionEnabled,
489495
getMetricsLinkedTimeEnabled,

tensorboard/webapp/metrics/store/metrics_selectors_test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
createTimeSeriesData,
2727
} from '../testing';
2828
import {HistogramMode, TooltipSort, XAxisType} from '../types';
29+
import {DataTableMode} from '../views/card_renderer/scalar_card_types';
2930
import * as selectors from './metrics_selectors';
3031
import {CardFeatureOverride, MetricsState} from './metrics_types';
3132

@@ -1473,4 +1474,19 @@ describe('metrics selectors', () => {
14731474
expect(selectors.isMetricsSettingsPaneOpen(state)).toEqual(false);
14741475
});
14751476
});
1477+
1478+
describe('#getTableEditorSelectedTab', () => {
1479+
beforeEach(() => {
1480+
selectors.getTableEditorSelectedTab.release();
1481+
});
1482+
1483+
it('returns current settings pane open state', () => {
1484+
const state = appStateFromMetricsState(
1485+
buildMetricsState({tableEditorSelectedTab: DataTableMode.RANGE})
1486+
);
1487+
expect(selectors.getTableEditorSelectedTab(state)).toEqual(
1488+
DataTableMode.RANGE
1489+
);
1490+
});
1491+
});
14761492
});

tensorboard/webapp/metrics/store/metrics_types.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ import {
3737
TooltipSort,
3838
XAxisType,
3939
} from '../types';
40-
import {ColumnHeader} from '../views/card_renderer/scalar_card_types';
40+
import {
41+
ColumnHeader,
42+
DataTableMode,
43+
} from '../views/card_renderer/scalar_card_types';
4144

4245
export const METRICS_FEATURE_KEY = 'metrics';
4346

@@ -244,6 +247,7 @@ export interface MetricsNonNamespacedState {
244247
timeSeriesData: TimeSeriesData;
245248
isSettingsPaneOpen: boolean;
246249
isSlideoutMenuOpen: boolean;
250+
tableEditorSelectedTab: DataTableMode;
247251
// Default settings. For the legacy reasons, we cannot change the name of the
248252
// prop. It either is set by application or a user via settings storage.
249253
settings: MetricsSettings;

tensorboard/webapp/metrics/testing.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
StepDatum,
4343
} from './store/metrics_types';
4444
import {CardId, CardMetadata, TooltipSort, XAxisType} from './types';
45+
import {DataTableMode} from './views/card_renderer/scalar_card_types';
4546

4647
export function buildMetricsSettingsState(
4748
overrides?: Partial<MetricsSettings>
@@ -110,6 +111,7 @@ function buildBlankState(): MetricsState {
110111
stepMinMax: {min: Infinity, max: -Infinity},
111112
isSettingsPaneOpen: false,
112113
isSlideoutMenuOpen: false,
114+
tableEditorSelectedTab: DataTableMode.SINGLE,
113115
};
114116
}
115117

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
<button
9797
*ngIf="columnCustomizationEnabled"
9898
mat-menu-item
99-
(click)="openSlideoutColumnEditMenu.emit()"
99+
(click)="openTableEditMenu()"
100100
aria-label="Open menu to edit data table columns"
101101
>
102102
<mat-icon svgIcon="edit_24px"></mat-icon>

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import {HeaderEditInfo, TooltipSort, XAxisType} from '../../types';
4848
import {
4949
ColumnHeader,
5050
ColumnHeaderType,
51+
DataTableMode,
5152
MinMaxStep,
5253
ScalarCardDataSeries,
5354
ScalarCardSeriesMetadata,
@@ -98,6 +99,7 @@ export class ScalarCardComponent<Downloader> {
9899
@Input() isProspectiveFobFeatureEnabled: Boolean = false;
99100
@Input() minMaxStep!: MinMaxStep;
100101
@Input() columnHeaders!: ColumnHeader[];
102+
@Input() rangeEnabled!: boolean;
101103

102104
@Output() onFullSizeToggle = new EventEmitter<void>();
103105
@Output() onPinClicked = new EventEmitter<boolean>();
@@ -109,7 +111,7 @@ export class ScalarCardComponent<Downloader> {
109111
new EventEmitter<TimeSelectionToggleAffordance>();
110112
@Output() onDataTableSorting = new EventEmitter<SortingInfo>();
111113
@Output() editColumnHeaders = new EventEmitter<HeaderEditInfo>();
112-
@Output() openSlideoutColumnEditMenu = new EventEmitter<void>();
114+
@Output() openTableEditMenuToMode = new EventEmitter<DataTableMode>();
113115

114116
@Output() onLineChartZoom = new EventEmitter<Extent>();
115117

@@ -282,4 +284,11 @@ export class ScalarCardComponent<Downloader> {
282284
this.dataTableContainer.nativeElement.style.height = '';
283285
}
284286
}
287+
288+
openTableEditMenu() {
289+
const currentTableMode = this.rangeEnabled
290+
? DataTableMode.RANGE
291+
: DataTableMode.SINGLE;
292+
this.openTableEditMenuToMode.emit(currentTableMode);
293+
}
285294
}

0 commit comments

Comments
 (0)