add saving-pins-checkbox to the settings view component

Author: roseayeon

tensorboard/webapp/metrics/views/right_pane/BUILD

@@ -25,9 +25,9 @@ tf_ng_module(
     srcs = [
         "right_pane_component.ts",
         "right_pane_module.ts",
+        "saving_pins_checkbox_component.ts",
         "settings_view_component.ts",
         "settings_view_container.ts",
-        "saving_pins_checkbox_component.ts",
     ],
     assets = [
         ":settings_view_styles",
@@ -40,12 +40,14 @@ tf_ng_module(
         "//tensorboard/webapp/angular:expect_angular_material_button",
         "//tensorboard/webapp/angular:expect_angular_material_button_toggle",
         "//tensorboard/webapp/angular:expect_angular_material_checkbox",
+        "//tensorboard/webapp/angular:expect_angular_material_dialog",
         "//tensorboard/webapp/angular:expect_angular_material_icon",
         "//tensorboard/webapp/angular:expect_angular_material_select",
         "//tensorboard/webapp/angular:expect_angular_material_slider",
         "//tensorboard/webapp/feature_flag",
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/actions",
+        "//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog",
         "//tensorboard/webapp/widgets/card_fob:types",
         "//tensorboard/webapp/widgets/dropdown",
         "//tensorboard/webapp/widgets/range_input",
@@ -71,14 +73,14 @@ tf_ts_library(
         "//tensorboard/webapp/angular:expect_angular_core_testing",
         "//tensorboard/webapp/angular:expect_angular_material_button_toggle",
         "//tensorboard/webapp/angular:expect_angular_material_checkbox",
-        "//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog",
         "//tensorboard/webapp/angular:expect_angular_material_select",
         "//tensorboard/webapp/angular:expect_angular_material_slider",
         "//tensorboard/webapp/angular:expect_angular_platform_browser_animations",
         "//tensorboard/webapp/angular:expect_ngrx_store_testing",
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/actions",
         "//tensorboard/webapp/metrics/store",
+        "//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog",
         "//tensorboard/webapp/testing:utils",
         "//tensorboard/webapp/widgets/card_fob:types",
         "//tensorboard/webapp/widgets/dropdown",

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

@@ -43,6 +43,7 @@ import {SavingPinsDialogModule} from './saving_pins_dialog/saving_pins_dialog_mo
     MatButtonModule,
     MatButtonToggleModule,
     MatCheckboxModule,
+    SavingPinsDialogModule,
     MatIconModule,
     MatSelectModule,
     MatSliderModule,

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

@@ -19,6 +19,7 @@ import {
   TestBed,
   tick,
 } from '@angular/core/testing';
+import {OverlayContainer} from '@angular/cdk/overlay';
 import {MatButtonToggleModule} from '@angular/material/button-toggle';
 import {MatCheckboxModule} from '@angular/material/checkbox';
 import {MatSelectModule} from '@angular/material/select';
@@ -37,10 +38,13 @@ import {HistogramMode, TooltipSort, XAxisType} from '../../types';
 import {RightPaneComponent} from './right_pane_component';
 import {SettingsViewComponent, TEST_ONLY} from './settings_view_component';
 import {SettingsViewContainer} from './settings_view_container';
+import {SavingPinsDialogModule} from './saving_pins_dialog/saving_pins_dialog_module';
+import {SavingPinsCheckboxComponent} from './saving_pins_checkbox_component';
 
 describe('metrics right_pane', () => {
   let store: MockStore<State>;
   let dispatchSpy: jasmine.Spy;
+  let overlayContainer: OverlayContainer;
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
@@ -49,13 +53,15 @@ describe('metrics right_pane', () => {
         DropdownModule,
         MatButtonToggleModule,
         MatCheckboxModule,
+        SavingPinsDialogModule,
         MatSelectModule,
         MatSliderModule,
       ],
       declarations: [
         RightPaneComponent,
         SettingsViewComponent,
         SettingsViewContainer,
+        SavingPinsCheckboxComponent,
       ],
       providers: [provideMockTbStore()],
       // Ignore errors from components that are out-of-scope for this test:
@@ -65,6 +71,7 @@ describe('metrics right_pane', () => {
 
     store = TestBed.inject<Store<State>>(Store) as MockStore<State>;
     dispatchSpy = spyOn(store, 'dispatch');
+    overlayContainer = TestBed.inject(OverlayContainer);
   });
 
   afterEach(() => {
@@ -574,17 +581,64 @@ describe('metrics right_pane', () => {
         ).toBeTrue();
       });
 
-      it('dispatches metricsEnableSavingPinsToggled on toggle', () => {
+      it('dispatches metricsEnableSavingPinsToggled if user checks the box', () => {
         const fixture = TestBed.createComponent(SettingsViewContainer);
         fixture.detectChanges();
 
         const checkbox = select(fixture, '.saving-pins input');
         checkbox.nativeElement.click();
 
+        const savingPinsDialog = overlayContainer
+          .getContainerElement()
+          .querySelectorAll('saving-pins-dialog');
+        expect(savingPinsDialog.length).toBe(0);
         expect(dispatchSpy).toHaveBeenCalledWith(
           actions.metricsEnableSavingPinsToggled()
         );
       });
+
+      it('dispatches metricsEnableSavingPinsToggled if users clicks disable in the dialog', async () => {
+        store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true);
+        const fixture = TestBed.createComponent(SettingsViewContainer);
+        fixture.detectChanges();
+
+        const checkbox = select(fixture, '.saving-pins input');
+        checkbox.nativeElement.click();
+
+        const savingPinsDialog = overlayContainer
+          .getContainerElement()
+          .querySelectorAll('saving-pins-dialog');
+
+        const disableButton = overlayContainer
+          .getContainerElement()
+          .querySelector('.disable-button');
+        (disableButton as HTMLButtonElement).click();
+        fixture.detectChanges();
+        await fixture.whenStable();
+
+        expect(savingPinsDialog.length).toBe(1);
+        expect(dispatchSpy).toHaveBeenCalledWith(
+          actions.metricsEnableSavingPinsToggled()
+        );
+      });
+
+      it('does not dispatch metricsEnableSavingPinsToggled if users cancels the dialog', async () => {
+        store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true);
+        const fixture = TestBed.createComponent(SettingsViewContainer);
+        fixture.detectChanges();
+
+        const checkbox = select(fixture, '.saving-pins input');
+        checkbox.nativeElement.click();
+
+        const disableButton = overlayContainer
+          .getContainerElement()
+          .querySelector('.cancel-button');
+        (disableButton as HTMLButtonElement).click();
+        fixture.detectChanges();
+        await fixture.whenStable();
+
+        expect(dispatchSpy).not.toHaveBeenCalled();
+      });
     });
   });
 });

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

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

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

@@ -14,6 +14,7 @@ limitations under the License.
 ==============================================================================*/
 import {ChangeDetectionStrategy, Component} from '@angular/core';
 import {Store} from '@ngrx/store';
+import {MatDialog} from '@angular/material/dialog';
 import {Observable} from 'rxjs';
 import {filter, map, take, withLatestFrom} from 'rxjs/operators';
 import {State} from '../../../app_state';
@@ -40,6 +41,10 @@ import {
   stepSelectorToggled,
 } from '../../actions';
 import {HistogramMode, TooltipSort, XAxisType} from '../../types';
+import {
+  SavingPinsDialogComponent,
+  SavingPinsDialogResult,
+} from './saving_pins_dialog/saving_pins_dialog_component';
 
 @Component({
   selector: 'metrics-dashboard-settings',
@@ -85,15 +90,18 @@ import {HistogramMode, TooltipSort, XAxisType} from '../../types';
       (rangeSelectionToggled)="onRangeSelectionToggled()"
       (onSlideOutToggled)="onSlideOutToggled()"
       [isSavingPinsEnabled]="isSavingPinsEnabled$ | async"
-      (onEnableSavingPinsToggled)="onEnableSavingPinsToggled()"
+      (onEnableSavingPinsToggled)="onEnableSavingPinsToggled($event)"
       [globalPinsFeatureEnabled]="globalPinsFeatureEnabled$ | async"
     >
     </metrics-dashboard-settings-component>
   `,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class SettingsViewContainer {
-  constructor(private readonly store: Store<State>) {}
+  constructor(
+    private readonly store: Store<State>,
+    private readonly dialog: MatDialog
+  ) {}
 
   readonly isScalarStepSelectorEnabled$: Observable<boolean> =
     this.store.select(selectors.getMetricsStepSelectorEnabled);
@@ -234,7 +242,17 @@ export class SettingsViewContainer {
     this.store.dispatch(metricsSlideoutMenuToggled());
   }
 
-  onEnableSavingPinsToggled() {
-    this.store.dispatch(metricsEnableSavingPinsToggled());
+  onEnableSavingPinsToggled(isChecked: boolean) {
+    if (isChecked) {
+      // Show a dialog when user disables the saving pins feature.
+      const dialogRef = this.dialog.open(SavingPinsDialogComponent);
+      dialogRef.afterClosed().subscribe((result?: SavingPinsDialogResult) => {
+        if (result?.shouldDisable) {
+          this.store.dispatch(metricsEnableSavingPinsToggled());
+        }
+      });
+    } else {
+      this.store.dispatch(metricsEnableSavingPinsToggled());
+    }
   }
 }