Skip to content

Commit af34605

Browse files
authored
[Global Pins] Shows a confirmation dialog when disabling the global pin feature (#6843)
## Motivation for features / changes Regarding #6831 , this PR adds confirmation dialog" that appears when the user clears the checkbox to make the user aware that local storage is disappearing. ## Technical description of changes 0e63b07 created a `saving pins dialog` component 05e06f5 Created a `saving pins checkbox` component * The Save Pin checkbox prevents the default click and must be done after confirmation in the modal. So I added `MAT_CHECKBOX_DEFAULT_OPTIONS` (https://v15.material.angular.io/comComponents/checkbox/overview#click-action-config) to prevent the default click event. However, if I add this directly to the `settings view component`, all checkboxes will be disabled, including other unwanted ones. So, I created a separate checkbox for the Save Pin checkbox. Please let me know is there any other good way to do this. 6ba778e Make the dialog shows up when user clears the checkbox * Passes to `onEnableSavingPinsToggled` whether the checkbox is selected. * If the checkbox is selected and user clicks it, it means that the user disables the global pin, so a dialog box is displayed. * If the checkbox is not checked but is clicked, it means the user is trying to enable a global pin and no dialog box will be displayed. * `onEnableSavingPinsToggled` and `isSavingPinsEnabled` is passed to `saving-pins-checkbox` component. ## Screenshots of UI changes (or N/A) ![image](https://github.com/tensorflow/tensorboard/assets/24772412/eb442565-4f39-45e7-b893-4866ee3fc1e4) ## Detailed steps to verify changes work correctly (as executed by you) unit test passes + TAP presubmit passes ## Alternate designs / implementations considered (or N/A)
1 parent ad1da0c commit af34605

15 files changed

+445
-16
lines changed

tensorboard/webapp/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ tf_ng_web_test_suite(
283283
"//tensorboard/webapp/metrics/views/card_renderer:card_renderer_tests",
284284
"//tensorboard/webapp/metrics/views/main_view:main_view_tests",
285285
"//tensorboard/webapp/metrics/views/right_pane:right_pane_test",
286+
"//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog:saving_pins_dialog_test",
286287
"//tensorboard/webapp/metrics/views/right_pane/scalar_column_editor:scalar_column_editor_test",
287288
"//tensorboard/webapp/plugins:plugins_container_test_lib",
288289
"//tensorboard/webapp/reloader:test_lib",

tensorboard/webapp/metrics/views/right_pane/BUILD

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,30 +11,43 @@ tf_sass_binary(
1111
],
1212
)
1313

14+
tf_sass_binary(
15+
name = "saving_pins_checkbox_styles",
16+
src = "saving_pins_checkbox_component.scss",
17+
deps = [
18+
"//tensorboard/webapp:angular_material_sass_deps",
19+
"//tensorboard/webapp/theme",
20+
],
21+
)
22+
1423
tf_ng_module(
1524
name = "right_pane",
1625
srcs = [
1726
"right_pane_component.ts",
1827
"right_pane_module.ts",
28+
"saving_pins_checkbox_component.ts",
1929
"settings_view_component.ts",
2030
"settings_view_container.ts",
2131
],
2232
assets = [
2333
":settings_view_styles",
2434
"settings_view_component.ng.html",
35+
":saving_pins_checkbox_styles",
2536
],
2637
deps = [
2738
"//tensorboard/webapp:app_state",
2839
"//tensorboard/webapp:selectors",
2940
"//tensorboard/webapp/angular:expect_angular_material_button",
3041
"//tensorboard/webapp/angular:expect_angular_material_button_toggle",
3142
"//tensorboard/webapp/angular:expect_angular_material_checkbox",
43+
"//tensorboard/webapp/angular:expect_angular_material_dialog",
3244
"//tensorboard/webapp/angular:expect_angular_material_icon",
3345
"//tensorboard/webapp/angular:expect_angular_material_select",
3446
"//tensorboard/webapp/angular:expect_angular_material_slider",
3547
"//tensorboard/webapp/feature_flag",
3648
"//tensorboard/webapp/metrics:types",
3749
"//tensorboard/webapp/metrics/actions",
50+
"//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog",
3851
"//tensorboard/webapp/widgets/card_fob:types",
3952
"//tensorboard/webapp/widgets/dropdown",
4053
"//tensorboard/webapp/widgets/range_input",
@@ -56,6 +69,7 @@ tf_ts_library(
5669
":right_pane",
5770
"//tensorboard/webapp:app_state",
5871
"//tensorboard/webapp:selectors",
72+
"//tensorboard/webapp/angular:expect_angular_cdk_overlay",
5973
"//tensorboard/webapp/angular:expect_angular_core_testing",
6074
"//tensorboard/webapp/angular:expect_angular_material_button_toggle",
6175
"//tensorboard/webapp/angular:expect_angular_material_checkbox",
@@ -66,6 +80,7 @@ tf_ts_library(
6680
"//tensorboard/webapp/metrics:types",
6781
"//tensorboard/webapp/metrics/actions",
6882
"//tensorboard/webapp/metrics/store",
83+
"//tensorboard/webapp/metrics/views/right_pane/saving_pins_dialog",
6984
"//tensorboard/webapp/testing:utils",
7085
"//tensorboard/webapp/widgets/card_fob:types",
7186
"//tensorboard/webapp/widgets/dropdown",

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@ import {RangeInputModule} from '../../../widgets/range_input/range_input_module'
2626
import {RightPaneComponent} from './right_pane_component';
2727
import {SettingsViewComponent} from './settings_view_component';
2828
import {SettingsViewContainer} from './settings_view_container';
29+
import {SavingPinsCheckboxComponent} from './saving_pins_checkbox_component';
30+
import {SavingPinsDialogModule} from './saving_pins_dialog/saving_pins_dialog_module';
2931

3032
@NgModule({
3133
declarations: [
3234
RightPaneComponent,
3335
SettingsViewComponent,
3436
SettingsViewContainer,
37+
SavingPinsCheckboxComponent,
3538
],
3639
exports: [RightPaneComponent],
3740
imports: [
@@ -40,6 +43,7 @@ import {SettingsViewContainer} from './settings_view_container';
4043
MatButtonModule,
4144
MatButtonToggleModule,
4245
MatCheckboxModule,
46+
SavingPinsDialogModule,
4347
MatIconModule,
4448
MatSelectModule,
4549
MatSliderModule,

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
TestBed,
2020
tick,
2121
} from '@angular/core/testing';
22+
import {OverlayContainer} from '@angular/cdk/overlay';
2223
import {MatButtonToggleModule} from '@angular/material/button-toggle';
2324
import {MatCheckboxModule} from '@angular/material/checkbox';
2425
import {MatSelectModule} from '@angular/material/select';
@@ -37,10 +38,13 @@ import {HistogramMode, TooltipSort, XAxisType} from '../../types';
3738
import {RightPaneComponent} from './right_pane_component';
3839
import {SettingsViewComponent, TEST_ONLY} from './settings_view_component';
3940
import {SettingsViewContainer} from './settings_view_container';
41+
import {SavingPinsDialogModule} from './saving_pins_dialog/saving_pins_dialog_module';
42+
import {SavingPinsCheckboxComponent} from './saving_pins_checkbox_component';
4043

4144
describe('metrics right_pane', () => {
4245
let store: MockStore<State>;
4346
let dispatchSpy: jasmine.Spy;
47+
let overlayContainer: OverlayContainer;
4448

4549
beforeEach(async () => {
4650
await TestBed.configureTestingModule({
@@ -49,13 +53,15 @@ describe('metrics right_pane', () => {
4953
DropdownModule,
5054
MatButtonToggleModule,
5155
MatCheckboxModule,
56+
SavingPinsDialogModule,
5257
MatSelectModule,
5358
MatSliderModule,
5459
],
5560
declarations: [
5661
RightPaneComponent,
5762
SettingsViewComponent,
5863
SettingsViewContainer,
64+
SavingPinsCheckboxComponent,
5965
],
6066
providers: [provideMockTbStore()],
6167
// Ignore errors from components that are out-of-scope for this test:
@@ -65,6 +71,7 @@ describe('metrics right_pane', () => {
6571

6672
store = TestBed.inject<Store<State>>(Store) as MockStore<State>;
6773
dispatchSpy = spyOn(store, 'dispatch');
74+
overlayContainer = TestBed.inject(OverlayContainer);
6875
});
6976

7077
afterEach(() => {
@@ -574,17 +581,64 @@ describe('metrics right_pane', () => {
574581
).toBeTrue();
575582
});
576583

577-
it('dispatches metricsEnableSavingPinsToggled on toggle', () => {
584+
it('dispatches metricsEnableSavingPinsToggled if user checks the box', () => {
578585
const fixture = TestBed.createComponent(SettingsViewContainer);
579586
fixture.detectChanges();
580587

581588
const checkbox = select(fixture, '.saving-pins input');
582589
checkbox.nativeElement.click();
583590

591+
const savingPinsDialog = overlayContainer
592+
.getContainerElement()
593+
.querySelectorAll('saving-pins-dialog');
594+
expect(savingPinsDialog.length).toBe(0);
584595
expect(dispatchSpy).toHaveBeenCalledWith(
585596
actions.metricsEnableSavingPinsToggled()
586597
);
587598
});
599+
600+
it('dispatches metricsEnableSavingPinsToggled if users clicks disable in the dialog', async () => {
601+
store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true);
602+
const fixture = TestBed.createComponent(SettingsViewContainer);
603+
fixture.detectChanges();
604+
605+
const checkbox = select(fixture, '.saving-pins input');
606+
checkbox.nativeElement.click();
607+
608+
const savingPinsDialog = overlayContainer
609+
.getContainerElement()
610+
.querySelectorAll('saving-pins-dialog');
611+
612+
const disableButton = overlayContainer
613+
.getContainerElement()
614+
.querySelector('.disable-button');
615+
(disableButton as HTMLButtonElement).click();
616+
fixture.detectChanges();
617+
await fixture.whenStable();
618+
619+
expect(savingPinsDialog.length).toBe(1);
620+
expect(dispatchSpy).toHaveBeenCalledWith(
621+
actions.metricsEnableSavingPinsToggled()
622+
);
623+
});
624+
625+
it('does not dispatch metricsEnableSavingPinsToggled if users cancels the dialog', async () => {
626+
store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true);
627+
const fixture = TestBed.createComponent(SettingsViewContainer);
628+
fixture.detectChanges();
629+
630+
const checkbox = select(fixture, '.saving-pins input');
631+
checkbox.nativeElement.click();
632+
633+
const disableButton = overlayContainer
634+
.getContainerElement()
635+
.querySelector('.cancel-button');
636+
(disableButton as HTMLButtonElement).click();
637+
fixture.detectChanges();
638+
await fixture.whenStable();
639+
640+
expect(dispatchSpy).not.toHaveBeenCalled();
641+
});
588642
});
589643
});
590644
});
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/* Copyright 2024 The TensorFlow Authors. All Rights Reserved.
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
==============================================================================*/
15+
@use '@angular/material' as mat;
16+
@import 'tensorboard/webapp/theme/tb_theme';
17+
18+
:host {
19+
@include tb-theme-foreground-prop(color, secondary-text);
20+
font-size: 12px;
21+
}
22+
23+
mat-checkbox {
24+
// Counteract the padding of the checkbox in order to align it vertically
25+
// with other items in the pane.
26+
margin-left: -11px;
27+
28+
::ng-deep label {
29+
@include tb-theme-foreground-prop(color, secondary-text);
30+
font-size: 12px;
31+
letter-spacing: normal;
32+
padding-left: 0px;
33+
white-space: nowrap;
34+
}
35+
}
36+
37+
.saving-pins-checkbox {
38+
align-items: center;
39+
display: flex;
40+
41+
.info {
42+
$_dim: 15px;
43+
height: $_dim;
44+
margin-left: 5px;
45+
width: $_dim;
46+
min-width: $_dim;
47+
}
48+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/* Copyright 2024 The TensorFlow Authors. All Rights Reserved.
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
==============================================================================*/
15+
import {
16+
ChangeDetectionStrategy,
17+
Component,
18+
EventEmitter,
19+
Input,
20+
Output,
21+
} from '@angular/core';
22+
import {
23+
MAT_CHECKBOX_DEFAULT_OPTIONS,
24+
MatCheckboxDefaultOptions,
25+
} from '@angular/material/checkbox';
26+
27+
@Component({
28+
selector: 'saving-pins-checkbox',
29+
template: `
30+
<div class="saving-pins-checkbox">
31+
<mat-checkbox [checked]="isChecked" (click)="onCheckboxToggled.emit()"
32+
>Enable saving pins (Scalars only)</mat-checkbox
33+
>
34+
<mat-icon
35+
class="info"
36+
svgIcon="help_outline_24px"
37+
title="When saving pins are enabled, pinned cards will be visible across multiple experiments."
38+
></mat-icon>
39+
</div>
40+
`,
41+
changeDetection: ChangeDetectionStrategy.OnPush,
42+
styleUrls: ['saving_pins_checkbox_component.css'],
43+
providers: [
44+
{
45+
provide: MAT_CHECKBOX_DEFAULT_OPTIONS,
46+
useValue: {clickAction: 'noop'} as MatCheckboxDefaultOptions,
47+
},
48+
],
49+
})
50+
export class SavingPinsCheckboxComponent {
51+
@Input() isChecked!: boolean;
52+
53+
@Output() onCheckboxToggled = new EventEmitter<void>();
54+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
load("//tensorboard/defs:defs.bzl", "tf_ng_module", "tf_sass_binary", "tf_ts_library")
2+
3+
package(default_visibility = ["//tensorboard:internal"])
4+
5+
tf_sass_binary(
6+
name = "saving_pins_dialog_component_styles",
7+
src = "saving_pins_dialog_component.scss",
8+
)
9+
10+
tf_ng_module(
11+
name = "saving_pins_dialog",
12+
srcs = [
13+
"saving_pins_dialog_component.ts",
14+
"saving_pins_dialog_module.ts",
15+
],
16+
assets = [
17+
":saving_pins_dialog_component_styles",
18+
"saving_pins_dialog_component.ng.html",
19+
],
20+
deps = [
21+
"//tensorboard/webapp/angular:expect_angular_material_button",
22+
"//tensorboard/webapp/angular:expect_angular_material_dialog",
23+
"@npm//@angular/common",
24+
"@npm//@angular/core",
25+
],
26+
)
27+
28+
tf_ts_library(
29+
name = "saving_pins_dialog_test",
30+
testonly = True,
31+
srcs = ["saving_pins_dialog_test.ts"],
32+
deps = [
33+
":saving_pins_dialog",
34+
"//tensorboard/webapp/angular:expect_angular_core_testing",
35+
"//tensorboard/webapp/angular:expect_angular_material_dialog",
36+
"@npm//@angular/common",
37+
"@npm//@angular/core",
38+
"@npm//@angular/platform-browser",
39+
"@npm//@types/jasmine",
40+
],
41+
)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!--
2+
@license
3+
Copyright 2024 The TensorFlow Authors. All Rights Reserved.
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
-->
17+
<h3 class="title">Disable Saving Pins?</h3>
18+
19+
<p>
20+
Disabling saving pins will remove locally stored pinned card data and no
21+
longer allow pinned cards to be shared across multiple experiments.<br />You
22+
can re-enable the feature and re-pin cards at any time.
23+
</p>
24+
25+
<div class="bottom-buttons">
26+
<button
27+
mat-flat-button
28+
(click)="closeWithoutDisabling()"
29+
class="cancel-button"
30+
>
31+
Cancel
32+
</button>
33+
<button mat-flat-button (click)="closeAndDisable()" class="disable-button">
34+
Disable
35+
</button>
36+
</div>

0 commit comments

Comments
 (0)