Skip to content

Commit 08294c4

Browse files
JamesHollyerJamesHollyer
authored
Column Customization: Persist Column Settings (#6253)
* Motivation for features / changes We now have a way for users to customize which columns they would like to appear in their data table and in what order. However, we currently revert back to default on every page load. This change stores their selection in localStorage so that it persists a crossed sessions. * Technical description of changes This just follows the same structure as all persisted settings in OSS.
1 parent dd8395b commit 08294c4

File tree

7 files changed

+156
-0
lines changed

7 files changed

+156
-0
lines changed

‎tensorboard/webapp/metrics/metrics_module.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
getMetricsScalarSmoothing,
3333
getMetricsStepSelectorEnabled,
3434
getMetricsTooltipSort,
35+
getRangeSelectionHeaders,
36+
getSingleSelectionHeaders,
3537
isMetricsSettingsPaneOpen,
3638
METRICS_FEATURE_KEY,
3739
METRICS_SETTINGS_DEFAULT,
@@ -115,6 +117,18 @@ export function getMetricsTimeSeriesLinkedTimeEnabled() {
115117
});
116118
}
117119

120+
export function getSingleSelectionHeadersFactory() {
121+
return createSelector(getSingleSelectionHeaders, (singleSelectionHeaders) => {
122+
return {singleSelectionHeaders};
123+
});
124+
}
125+
126+
export function getRangeSelectionHeadersFactory() {
127+
return createSelector(getRangeSelectionHeaders, (rangeSelectionHeaders) => {
128+
return {rangeSelectionHeaders};
129+
});
130+
}
131+
118132
@NgModule({
119133
imports: [
120134
CommonModule,
@@ -157,6 +171,12 @@ export function getMetricsTimeSeriesLinkedTimeEnabled() {
157171
PersistentSettingsConfigModule.defineGlobalSetting(
158172
getMetricsTimeSeriesLinkedTimeEnabled
159173
),
174+
PersistentSettingsConfigModule.defineGlobalSetting(
175+
getSingleSelectionHeadersFactory
176+
),
177+
PersistentSettingsConfigModule.defineGlobalSetting(
178+
getRangeSelectionHeadersFactory
179+
),
160180
],
161181
providers: [
162182
{

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,13 +461,19 @@ const reducer = createReducer(
461461
partialSettings.rangeSelectionEnabled ?? state.rangeSelectionEnabled;
462462
const linkedTimeEnabled =
463463
partialSettings.linkedTimeEnabled ?? state.linkedTimeEnabled;
464+
const singleSelectionHeaders =
465+
partialSettings.singleSelectionHeaders ?? state.singleSelectionHeaders;
466+
const rangeSelectionHeaders =
467+
partialSettings.rangeSelectionHeaders ?? state.rangeSelectionHeaders;
464468

465469
return {
466470
...state,
467471
isSettingsPaneOpen,
468472
stepSelectorEnabled,
469473
rangeSelectionEnabled,
470474
linkedTimeEnabled,
475+
singleSelectionHeaders,
476+
rangeSelectionHeaders,
471477
settings: {
472478
...state.settings,
473479
...metricsSettings,

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2804,6 +2804,67 @@ describe('metrics reducers', () => {
28042804

28052805
expect(nextState.isSettingsPaneOpen).toBe(false);
28062806
});
2807+
2808+
it('loads singleSelectionHeaders setting into the next state', () => {
2809+
const beforeState = buildMetricsState({
2810+
singleSelectionHeaders: [
2811+
{type: ColumnHeaderType.RUN, enabled: true},
2812+
{type: ColumnHeaderType.SMOOTHED, enabled: true},
2813+
{type: ColumnHeaderType.VALUE, enabled: true},
2814+
],
2815+
});
2816+
2817+
const nextState = reducers(
2818+
beforeState,
2819+
globalSettingsLoaded({
2820+
partialSettings: {
2821+
singleSelectionHeaders: [
2822+
{type: ColumnHeaderType.SMOOTHED, enabled: true},
2823+
{type: ColumnHeaderType.RUN, enabled: true},
2824+
{type: ColumnHeaderType.VALUE, enabled: false},
2825+
],
2826+
},
2827+
})
2828+
);
2829+
2830+
expect(nextState.singleSelectionHeaders).toEqual([
2831+
{type: ColumnHeaderType.SMOOTHED, enabled: true},
2832+
{type: ColumnHeaderType.RUN, enabled: true},
2833+
{type: ColumnHeaderType.VALUE, enabled: false},
2834+
]);
2835+
});
2836+
2837+
it('loads rangeSelectionHeaders setting into the next state', () => {
2838+
const beforeState = buildMetricsState({
2839+
rangeSelectionHeaders: [
2840+
{type: ColumnHeaderType.RUN, enabled: true},
2841+
{type: ColumnHeaderType.MIN_VALUE, enabled: true},
2842+
{type: ColumnHeaderType.MAX_VALUE, enabled: true},
2843+
{type: ColumnHeaderType.MEAN, enabled: false},
2844+
],
2845+
});
2846+
2847+
const nextState = reducers(
2848+
beforeState,
2849+
globalSettingsLoaded({
2850+
partialSettings: {
2851+
rangeSelectionHeaders: [
2852+
{type: ColumnHeaderType.RUN, enabled: true},
2853+
{type: ColumnHeaderType.MEAN, enabled: true},
2854+
{type: ColumnHeaderType.MAX_VALUE, enabled: true},
2855+
{type: ColumnHeaderType.MIN_VALUE, enabled: false},
2856+
],
2857+
},
2858+
})
2859+
);
2860+
2861+
expect(nextState.rangeSelectionHeaders).toEqual([
2862+
{type: ColumnHeaderType.RUN, enabled: true},
2863+
{type: ColumnHeaderType.MEAN, enabled: true},
2864+
{type: ColumnHeaderType.MAX_VALUE, enabled: true},
2865+
{type: ColumnHeaderType.MIN_VALUE, enabled: false},
2866+
]);
2867+
});
28072868
});
28082869

28092870
it('loads Step Selector Setting into the next state', () => {

‎tensorboard/webapp/persistent_settings/_data_source/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ tf_ng_module(
2424
],
2525
deps = [
2626
"//tensorboard/webapp/metrics:types",
27+
"//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types",
2728
],
2829
)
2930

@@ -53,6 +54,7 @@ tf_ng_module(
5354
":types",
5455
"//tensorboard/webapp/angular:expect_angular_core_testing",
5556
"//tensorboard/webapp/metrics:types",
57+
"//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types",
5658
"@npm//@angular/core",
5759
"@npm//@types/jasmine",
5860
"@npm//rxjs",

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ export class OSSSettingsConverter extends SettingsConverter<
9494
if (settings.linkedTimeEnabled !== undefined) {
9595
serializableSettings.linkedTimeEnabled = settings.linkedTimeEnabled;
9696
}
97+
if (settings.singleSelectionHeaders !== undefined) {
98+
serializableSettings.singleSelectionHeaders =
99+
settings.singleSelectionHeaders;
100+
}
101+
if (settings.rangeSelectionHeaders !== undefined) {
102+
serializableSettings.rangeSelectionHeaders =
103+
settings.rangeSelectionHeaders;
104+
}
97105
return serializableSettings;
98106
}
99107

@@ -200,6 +208,20 @@ export class OSSSettingsConverter extends SettingsConverter<
200208
settings.linkedTimeEnabled = backendSettings.linkedTimeEnabled;
201209
}
202210

211+
if (
212+
backendSettings.hasOwnProperty('singleSelectionHeaders') &&
213+
typeof backendSettings.singleSelectionHeaders === 'object'
214+
) {
215+
settings.singleSelectionHeaders = backendSettings.singleSelectionHeaders;
216+
}
217+
218+
if (
219+
backendSettings.hasOwnProperty('rangeSelectionHeaders') &&
220+
typeof backendSettings.rangeSelectionHeaders === 'object'
221+
) {
222+
settings.rangeSelectionHeaders = backendSettings.rangeSelectionHeaders;
223+
}
224+
203225
return settings;
204226
}
205227
}
@@ -263,6 +285,7 @@ export class PersistentSettingsDataSourceImpl<UiSettings, StorageSettings>
263285
localStorage.getItem(LEGACY_METRICS_LOCAL_STORAGE_KEY) ?? '{}'
264286
)
265287
);
288+
266289
const settings = this.converter.backendToUi(
267290
this.deserialize(localStorage.getItem(GLOBAL_LOCAL_STORAGE_KEY) ?? '{}')
268291
);

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {Injectable} from '@angular/core';
1616
import {TestBed} from '@angular/core/testing';
1717
import {firstValueFrom} from 'rxjs';
1818
import {TooltipSort} from '../../metrics/types';
19+
import {ColumnHeaderType} from '../../metrics/views/card_renderer/scalar_card_types';
1920
import {
2021
OSSSettingsConverter,
2122
PersistentSettingsDataSourceImpl,
@@ -182,6 +183,44 @@ describe('persistent_settings data_source test', () => {
182183
linkedTimeEnabled: true,
183184
});
184185
});
186+
it('properly converts singleSelectionEnabled', async () => {
187+
getItemSpy.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY).and.returnValue(
188+
JSON.stringify({
189+
singleSelectionHeaders: [
190+
{type: ColumnHeaderType.RUN, enabled: true},
191+
{type: ColumnHeaderType.VALUE, enabled: false},
192+
],
193+
})
194+
);
195+
196+
const actual = await firstValueFrom(dataSource.getSettings());
197+
198+
expect(actual).toEqual({
199+
singleSelectionHeaders: [
200+
{type: ColumnHeaderType.RUN, enabled: true},
201+
{type: ColumnHeaderType.VALUE, enabled: false},
202+
],
203+
});
204+
});
205+
it('properly converts rangeSelectionEnabled', async () => {
206+
getItemSpy.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY).and.returnValue(
207+
JSON.stringify({
208+
rangeSelectionHeaders: [
209+
{type: ColumnHeaderType.RUN, enabled: true},
210+
{type: ColumnHeaderType.MIN_VALUE, enabled: true},
211+
],
212+
})
213+
);
214+
215+
const actual = await firstValueFrom(dataSource.getSettings());
216+
217+
expect(actual).toEqual({
218+
rangeSelectionHeaders: [
219+
{type: ColumnHeaderType.RUN, enabled: true},
220+
{type: ColumnHeaderType.MIN_VALUE, enabled: true},
221+
],
222+
});
223+
});
185224
});
186225

187226
describe('#setSettings', () => {

‎tensorboard/webapp/persistent_settings/_data_source/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ limitations under the License.
1414
==============================================================================*/
1515

1616
import {TooltipSort} from '../../metrics/types';
17+
import {ColumnHeader} from '../../metrics/views/card_renderer/scalar_card_types';
1718

1819
export enum ThemeValue {
1920
BROWSER_DEFAULT = 'browser_default',
@@ -43,6 +44,8 @@ export declare interface BackendSettings {
4344
stepSelectorEnabled?: boolean;
4445
rangeSelectionEnabled?: boolean;
4546
linkedTimeEnabled?: boolean;
47+
singleSelectionHeaders?: ColumnHeader[];
48+
rangeSelectionHeaders?: ColumnHeader[];
4649
}
4750

4851
/**
@@ -65,4 +68,6 @@ export interface PersistableSettings {
6568
stepSelectorEnabled?: boolean;
6669
rangeSelectionEnabled?: boolean;
6770
linkedTimeEnabled?: boolean;
71+
singleSelectionHeaders?: ColumnHeader[];
72+
rangeSelectionHeaders?: ColumnHeader[];
6873
}

0 commit comments

Comments
 (0)