[Global Pins] Added a disabling global pins feature (tensorflow#6831)

## Motivation for features / changes
Following tensorflow#6828,

Since global pins are applied automatically, this PR allows users
disable the global pin feature.

## Technical description of changes
2cf2b11 Added `savingPinsEnabled` in the `MetricsSettings`
* Also added a selector getting `settings.savingPinsEnabled` value from
the store.

dec6a63 Introduced new action `metricsEnableSavingPinsToggled`
* If `metricsEnableSavingPinsToggled` is dispatched, it toggles the
`savingPinsEnabled` store value.

6ea22fd Added a checkbox UI to the settings view component.
* Also added a feature flag guard to the component. If check box is
clicked, it dispatched `metricsEnableSavingPinsToggled` action.

568febf Added a new effect `disableSavingPins`
* When `metricsEnableSavingPinsToggled` action is called, this effect
will call `removeAllScalarPins` method if `savingPinsEnabled` value is
false.
 
bbf9050 Added saving pins feature in persistent settings to preserve
user preferences.

3ff7cd2 Add buildMetricsSettingsOverrides util in the testing to use in
tbcorp

## Screenshots of UI changes (or N/A)

![image](https://github.com/tensorflow/tensorboard/assets/24772412/874dbc2b-f01f-4112-b34b-6b07330ea31f)

## Detailed steps to verify changes work correctly (as executed by you)
Unit test passes + created a cl/625208577 to pass TAP Presubmit tests
## Alternate designs / implementations considered (or N/A)
N/A

## Action Items in the next PR
In the next PR, I'll add a "confirmation dialog" that appears when the
user unchecks the checkbox to make the user aware that local storage is
disappearing.

- [ ] Add the confirmation dialog

Author: roseayeon

tensorboard/webapp/metrics/actions/index.ts

@@ -281,5 +281,9 @@ export const metricsClearAllPinnedCards = createAction(
   '[Metrics] Clear all pinned cards'
 );
 
+export const metricsEnableSavingPinsToggled = createAction(
+  '[Metrics] Enable Saving Pins Toggled'
+);
+
 // TODO(jieweiwu): Delete after internal code is updated.
 export const stepSelectorTimeSelectionChanged = timeSelectionChanged;

tensorboard/webapp/metrics/effects/index.ts

@@ -268,11 +268,20 @@ export class MetricsEffects implements OnInitEffects {
     withLatestFrom(
       this.getVisibleCardFetchInfos(),
       this.store.select(selectors.getEnableGlobalPins),
-      this.store.select(selectors.getShouldPersistSettings)
+      this.store.select(selectors.getShouldPersistSettings),
+      this.store.select(selectors.getMetricsSavingPinsEnabled)
     ),
     filter(
-      ([, , enableGlobalPins, shouldPersistSettings]) =>
-        enableGlobalPins && shouldPersistSettings
+      ([
+        ,
+        ,
+        enableGlobalPinsFeature,
+        shouldPersistSettings,
+        isMetricsSavingPinsEnabled,
+      ]) =>
+        enableGlobalPinsFeature &&
+        shouldPersistSettings &&
+        isMetricsSavingPinsEnabled
     ),
     tap(([{cardId, canCreateNewPins, wasPinned}, fetchInfos]) => {
       const card = fetchInfos.find((value) => value.id === cardId);
@@ -293,11 +302,19 @@ export class MetricsEffects implements OnInitEffects {
     ofType(initAction),
     withLatestFrom(
       this.store.select(selectors.getEnableGlobalPins),
-      this.store.select(selectors.getShouldPersistSettings)
+      this.store.select(selectors.getShouldPersistSettings),
+      this.store.select(selectors.getMetricsSavingPinsEnabled)
     ),
     filter(
-      ([, enableGlobalPins, shouldPersistSettings]) =>
-        enableGlobalPins && shouldPersistSettings
+      ([
+        ,
+        enableGlobalPinsFeature,
+        shouldPersistSettings,
+        isMetricsSavingPinsEnabled,
+      ]) =>
+        enableGlobalPinsFeature &&
+        shouldPersistSettings &&
+        isMetricsSavingPinsEnabled
     ),
     tap(() => {
       const tags = this.savedPinsDataSource.getSavedScalarPins();
@@ -316,15 +333,46 @@ export class MetricsEffects implements OnInitEffects {
     })
   );
 
-  private readonly removeAllPins$ = this.actions$.pipe(
+  private readonly removeSavedPinsOnDisable$ = this.actions$.pipe(
     ofType(actions.metricsClearAllPinnedCards),
     withLatestFrom(
       this.store.select(selectors.getEnableGlobalPins),
-      this.store.select(selectors.getShouldPersistSettings)
+      this.store.select(selectors.getShouldPersistSettings),
+      this.store.select(selectors.getMetricsSavingPinsEnabled)
     ),
     filter(
-      ([, enableGlobalPins, shouldPersistSettings]) =>
-        enableGlobalPins && shouldPersistSettings
+      ([
+        ,
+        enableGlobalPinsFeature,
+        shouldPersistSettings,
+        isMetricsSavingPinsEnabled,
+      ]) =>
+        enableGlobalPinsFeature &&
+        shouldPersistSettings &&
+        isMetricsSavingPinsEnabled
+    ),
+    tap(() => {
+      this.savedPinsDataSource.removeAllScalarPins();
+    })
+  );
+
+  private readonly disableSavingPins$ = this.actions$.pipe(
+    ofType(actions.metricsEnableSavingPinsToggled),
+    withLatestFrom(
+      this.store.select(selectors.getEnableGlobalPins),
+      this.store.select(selectors.getShouldPersistSettings),
+      this.store.select(selectors.getMetricsSavingPinsEnabled)
+    ),
+    filter(
+      ([
+        ,
+        enableGlobalPins,
+        getShouldPersistSettings,
+        getMetricsSavingPinsEnabled,
+      ]) =>
+        enableGlobalPins &&
+        getShouldPersistSettings &&
+        !getMetricsSavingPinsEnabled
     ),
     tap(() => {
       this.savedPinsDataSource.removeAllScalarPins();
@@ -375,7 +423,7 @@ export class MetricsEffects implements OnInitEffects {
         /**
          * Subscribes to: metricsClearAllPinnedCards.
          */
-        this.removeAllPins$
+        this.removeSavedPinsOnDisable$
       );
     },
     {dispatch: false}

tensorboard/webapp/metrics/metrics_module.ts

@@ -35,6 +35,7 @@ import {
   getMetricsScalarSmoothing,
   getMetricsStepSelectorEnabled,
   getMetricsTooltipSort,
+  getMetricsSavingPinsEnabled,
   getRangeSelectionHeaders,
   getSingleSelectionHeaders,
   isMetricsSettingsPaneOpen,
@@ -125,6 +126,12 @@ export function getMetricsTimeSeriesLinkedTimeEnabled() {
   });
 }
 
+export function getMetricsTimeSeriesSavingPinsEnabled() {
+  return createSelector(getMetricsSavingPinsEnabled, (isEnabled) => {
+    return {savingPinsEnabled: isEnabled};
+  });
+}
+
 export function getSingleSelectionHeadersFactory() {
   return createSelector(getSingleSelectionHeaders, (singleSelectionHeaders) => {
     return {singleSelectionHeaders};
@@ -188,6 +195,9 @@ export function getRangeSelectionHeadersFactory() {
     PersistentSettingsConfigModule.defineGlobalSetting(
       getRangeSelectionHeadersFactory
     ),
+    PersistentSettingsConfigModule.defineGlobalSetting(
+      getMetricsTimeSeriesSavingPinsEnabled
+    ),
   ],
   providers: [
     {

tensorboard/webapp/metrics/store/metrics_reducers.ts

@@ -608,6 +608,9 @@ const reducer = createReducer(
     if (typeof partialSettings.scalarSmoothing === 'number') {
       metricsSettings.scalarSmoothing = partialSettings.scalarSmoothing;
     }
+    if (typeof partialSettings.savingPinsEnabled === 'boolean') {
+      metricsSettings.savingPinsEnabled = partialSettings.savingPinsEnabled;
+    }
 
     const isSettingsPaneOpen =
       partialSettings.timeSeriesSettingsPaneOpened ?? state.isSettingsPaneOpen;
@@ -933,6 +936,19 @@ const reducer = createReducer(
       },
     };
   }),
+  on(actions.metricsEnableSavingPinsToggled, (state) => {
+    const nextSavingPinsEnabled = !(
+      state.settingOverrides.savingPinsEnabled ??
+      state.settings.savingPinsEnabled
+    );
+    return {
+      ...state,
+      settingOverrides: {
+        ...state.settingOverrides,
+        savingPinsEnabled: nextSavingPinsEnabled,
+      },
+    };
+  }),
   on(
     actions.multipleTimeSeriesRequested,
     (

tensorboard/webapp/metrics/store/metrics_reducers_test.ts

@@ -1232,6 +1232,25 @@ describe('metrics reducers', () => {
       expect(thirdState.settings.hideEmptyCards).toBe(false);
       expect(thirdState.settingOverrides.hideEmptyCards).toBe(false);
     });
+
+    it('changes savingPinsEnabled on metricsEnableSavingPinsToggled', () => {
+      [{value: true}, {value: false}].forEach(({value: initValue}) => {
+        const prevState = buildMetricsState({
+          settings: buildMetricsSettingsState({
+            savingPinsEnabled: initValue,
+          }),
+          settingOverrides: {},
+        });
+
+        const nextState = reducers(
+          prevState,
+          actions.metricsEnableSavingPinsToggled()
+        );
+
+        expect(nextState.settings.savingPinsEnabled).toBe(initValue);
+        expect(nextState.settingOverrides.savingPinsEnabled).toBe(!initValue);
+      });
+    });
   });
 
   describe('loading time series data', () => {
@@ -3142,6 +3161,7 @@ describe('metrics reducers', () => {
           scalarSmoothing: 0.3,
           ignoreOutliers: false,
           tooltipSort: TooltipSort.ASCENDING,
+          savingPinsEnabled: true,
         }),
         settingOverrides: {
           scalarSmoothing: 0.5,
@@ -3155,13 +3175,15 @@ describe('metrics reducers', () => {
           partialSettings: {
             ignoreOutliers: true,
             tooltipSort: TooltipSort.DESCENDING,
+            savingPinsEnabled: false,
           },
         })
       );
 
       expect(nextState.settings.scalarSmoothing).toBe(0.3);
       expect(nextState.settings.ignoreOutliers).toBe(true);
       expect(nextState.settings.tooltipSort).toBe(TooltipSort.DESCENDING);
+      expect(nextState.settings.savingPinsEnabled).toBe(false);
       expect(nextState.settingOverrides.scalarSmoothing).toBe(0.5);
       expect(nextState.settingOverrides.tooltipSort).toBe(
         TooltipSort.ALPHABETICAL

tensorboard/webapp/metrics/store/metrics_selectors.ts

@@ -382,6 +382,11 @@ export const getMetricsImageShowActualSize = createSelector(
   (settings): boolean => settings.imageShowActualSize
 );
 
+export const getMetricsSavingPinsEnabled = createSelector(
+  selectSettings,
+  (settings): boolean => settings.savingPinsEnabled
+);
+
 export const getMetricsTagFilter = createSelector(
   selectMetricsState,
   (state): string => state.tagFilter

tensorboard/webapp/metrics/store/metrics_selectors_test.ts

@@ -1298,6 +1298,20 @@ describe('metrics selectors', () => {
       );
       expect(selectors.getMetricsCardMinWidth(state)).toBe(400);
     });
+
+    it('returns savingPinsEnabled when called getMetricsSavingPinsEnabled', () => {
+      [{value: true}, {value: false}].forEach(({value}) => {
+        selectors.getMetricsSavingPinsEnabled.release();
+        const state = appStateFromMetricsState(
+          buildMetricsState({
+            settings: buildMetricsSettingsState({
+              savingPinsEnabled: value,
+            }),
+          })
+        );
+        expect(selectors.getMetricsSavingPinsEnabled(state)).toBe(value);
+      });
+    });
   });
 
   describe('getMetricsTagFilter', () => {

tensorboard/webapp/metrics/store/metrics_types.ts

@@ -246,6 +246,7 @@ export interface MetricsSettings {
   imageContrastInMilli: number;
   imageShowActualSize: boolean;
   histogramMode: HistogramMode;
+  savingPinsEnabled: boolean;
 }
 
 export interface MetricsNonNamespacedState {
@@ -287,4 +288,5 @@ export const METRICS_SETTINGS_DEFAULT: MetricsSettings = {
   imageContrastInMilli: 1000,
   imageShowActualSize: false,
   histogramMode: HistogramMode.OFFSET,
+  savingPinsEnabled: true,
 };

tensorboard/webapp/metrics/testing.ts

@@ -47,6 +47,28 @@ import {DataTableMode} from '../widgets/data_table/types';
 export function buildMetricsSettingsState(
   overrides?: Partial<MetricsSettings>
 ): MetricsSettings {
+  return {
+    cardMinWidth: null,
+    tooltipSort: TooltipSort.NEAREST,
+    ignoreOutliers: false,
+    xAxisType: XAxisType.WALL_TIME,
+    scalarSmoothing: 0.3,
+    hideEmptyCards: true,
+    scalarPartitionNonMonotonicX: false,
+    imageBrightnessInMilli: 123,
+    imageContrastInMilli: 123,
+    imageShowActualSize: true,
+    histogramMode: HistogramMode.OFFSET,
+    savingPinsEnabled: true,
+    ...overrides,
+  };
+}
+
+// Since Settings proto has missing fields, we need to build a partial of
+// Settings to be used in tests.
+export function buildMetricsSettingsOverrides(
+  overrides?: Partial<MetricsSettings>
+): Partial<MetricsSettings> {
   return {
     cardMinWidth: null,
     tooltipSort: TooltipSort.NEAREST,

tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts

@@ -548,5 +548,43 @@ describe('metrics right_pane', () => {
         ).toHaveClass('toggle-opened');
       });
     });
+
+    describe('saving pins check box', () => {
+      beforeEach(() => {
+        store.overrideSelector(selectors.getEnableGlobalPins, true);
+        store.overrideSelector(selectors.getMetricsSavingPinsEnabled, false);
+      });
+
+      it('does not render if getEnableGlobalPins feature flag is false', () => {
+        store.overrideSelector(selectors.getEnableGlobalPins, false);
+        const fixture = TestBed.createComponent(SettingsViewContainer);
+        fixture.detectChanges();
+
+        expect(select(fixture, '.saving-pins')).toBeFalsy();
+      });
+
+      it('renders checked saving pins check box if isSavingpinsEnabled is true', () => {
+        store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true);
+
+        const fixture = TestBed.createComponent(SettingsViewContainer);
+        fixture.detectChanges();
+
+        expect(
+          select(fixture, '.saving-pins input').componentInstance.checked
+        ).toBeTrue();
+      });
+
+      it('dispatches metricsEnableSavingPinsToggled on toggle', () => {
+        const fixture = TestBed.createComponent(SettingsViewContainer);
+        fixture.detectChanges();
+
+        const checkbox = select(fixture, '.saving-pins input');
+        checkbox.nativeElement.click();
+
+        expect(dispatchSpy).toHaveBeenCalledWith(
+          actions.metricsEnableSavingPinsToggled()
+        );
+      });
+    });
   });
 });

tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html

@@ -93,6 +93,19 @@ <h3 class="section-title">General</h3>
       </button>
     </div>
   </div>
+
+  <div class="control-row saving-pins" *ngIf="globalPinsFeatureEnabled">
+    <mat-checkbox
+      [checked]="isSavingPinsEnabled"
+      (change)="onEnableSavingPinsToggled.emit()"
+      >Enable saving pins (Scalars only)</mat-checkbox
+    >
+    <mat-icon
+      class="info"
+      svgIcon="help_outline_24px"
+      title="When saving pins are enabled, pinned cards will be visible across multiple experiments."
+    ></mat-icon>
+  </div>
 </section>
 
 <section class="scalars">

tensorboard/webapp/metrics/views/right_pane/settings_view_component.scss

@@ -79,7 +79,8 @@ section .control-row:not(:has(+ .control-row > mat-checkbox)):not(:last-child) {
   width: 5em;
 }
 
-.scalars-partition-x {
+.scalars-partition-x,
+.saving-pins {
   align-items: center;
   display: flex;
 

tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts

@@ -70,12 +70,15 @@ export class SettingsViewComponent {
   @Input() linkedTimeSelection!: TimeSelection | null;
   @Input() stepMinMax!: {min: number; max: number};
   @Input() isSlideOutMenuOpen!: boolean;
+  @Input() isSavingPinsEnabled!: boolean;
+  @Input() globalPinsFeatureEnabled: boolean = false;
 
   @Output() linkedTimeToggled = new EventEmitter<void>();
 
   @Output() stepSelectorToggled = new EventEmitter<void>();
   @Output() rangeSelectionToggled = new EventEmitter<void>();
   @Output() onSlideOutToggled = new EventEmitter<void>();
+  @Output() onEnableSavingPinsToggled = new EventEmitter<void>();
 
   @Input() isImageSupportEnabled!: boolean;