linked time: add affordance for toggling action (tensorflow#5822)

We are interested in learning how often deselect is trigged. However, when deselecting fob in single selection, we triggered toggle action instead of time selection change. We do not know if the toggle action is trigged by deselecting fob or check/uncheck box in settings pane. This PR adds an affordance to the action.
dna2fork · Jul 27, 2022 · 0f4d6e9 · 0f4d6e9
1 parent cf342b0
commit 0f4d6e9
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 36 deletions.
diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts
@@ -14,7 +14,10 @@ limitations under the License.
 ==============================================================================*/
 import {createAction, props} from '@ngrx/store';
 import {ElementId} from '../../util/dom';
-import {TimeSelectionAffordance} from '../../widgets/card_fob/card_fob_types';
+import {
+  TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
+} from '../../widgets/card_fob/card_fob_types';
 import {
   TagMetadata,
   TimeSeriesRequest,
@@ -177,7 +180,7 @@ export const linkedTimeSelectionChanged = createAction(
       startStep: number;
       endStep: number | undefined;
     };
-    // Affordance for analytics purpose. When no affordance is specified or is
+    // Affordance for internal analytics purpose. When no affordance is specified or is
     // undefined we do not want to log an analytics event.
     affordance?: TimeSelectionAffordance | undefined;
   }>()
@@ -188,11 +191,21 @@ export const timeSelectionCleared = createAction(
 );
 
 export const linkedTimeToggled = createAction(
-  '[Metrics] Linked Time Enable Toggle'
+  '[Metrics] Linked Time Enable Toggle',
+  props<{
+    // Affordance for internal analytics purpose. When no affordance is specified or is
+    // undefined we do not want to log an analytics event.
+    affordance?: TimeSelectionToggleAffordance;
+  }>()
 );
 
 export const stepSelectorToggled = createAction(
-  '[Metrics] Time Selector Enable Toggle'
+  '[Metrics] Time Selector Enable Toggle',
+  props<{
+    // Affordance for internal analytics purpose. When no affordance is specified or is
+    // undefined we do not want to log an analytics event.
+    affordance?: TimeSelectionToggleAffordance;
+  }>()
 );
 
 /**

diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts
@@ -2737,10 +2737,10 @@ describe('metrics reducers', () => {
           linkedTimeEnabled: false,
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
         expect(state2.linkedTimeEnabled).toBe(true);
 
-        const state3 = reducers(state2, actions.linkedTimeToggled());
+        const state3 = reducers(state2, actions.linkedTimeToggled({}));
         expect(state3.linkedTimeEnabled).toBe(false);
       });
 
@@ -2770,7 +2770,7 @@ describe('metrics reducers', () => {
           cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
 
         expect(state2.cardStepIndex).toEqual({
           [imageCardId]: buildStepIndexMetadata({index: 0}),
@@ -2802,7 +2802,7 @@ describe('metrics reducers', () => {
           cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
         expect(state2.cardStepIndex).toEqual({
           [imageCardId]: buildStepIndexMetadata({index: 1}),
         });
@@ -2840,7 +2840,7 @@ describe('metrics reducers', () => {
           cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
         expect(state2.cardStepIndex).toEqual({
           [imageCardId]: buildStepIndexMetadata({index: 2}),
         });
@@ -2851,7 +2851,7 @@ describe('metrics reducers', () => {
           stepMinMax: {min: Infinity, max: -Infinity},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
 
         expect(state2.linkedTimeSelection).toEqual({
           start: {step: 0},
@@ -2864,7 +2864,7 @@ describe('metrics reducers', () => {
           stepMinMax: {min: 10, max: 100},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
 
         expect(state2.linkedTimeSelection).toEqual({
           start: {step: 10},
@@ -2877,7 +2877,7 @@ describe('metrics reducers', () => {
           linkedTimeSelection: {start: {step: 20}, end: null},
         });
 
-        const state2 = reducers(state1, actions.linkedTimeToggled());
+        const state2 = reducers(state1, actions.linkedTimeToggled({}));
         expect(state2.linkedTimeSelection).toEqual({
           start: {step: 20},
           end: null,
@@ -2892,10 +2892,10 @@ describe('metrics reducers', () => {
         stepSelectorEnabled: false,
       });
 
-      const state2 = reducers(state1, actions.stepSelectorToggled());
+      const state2 = reducers(state1, actions.stepSelectorToggled({}));
       expect(state2.stepSelectorEnabled).toBe(true);
 
-      const state3 = reducers(state2, actions.stepSelectorToggled());
+      const state3 = reducers(state2, actions.stepSelectorToggled({}));
       expect(state3.stepSelectorEnabled).toBe(false);
     });
   });

diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts
@@ -26,7 +26,10 @@ import {filter, map} from 'rxjs/operators';
 import {State} from '../../../app_state';
 import {DataLoadState} from '../../../types/data';
 import {RunColorScale} from '../../../types/ui';
-import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
+import {
+  TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
+} from '../../../widgets/card_fob/card_fob_types';
 import {HistogramDatum} from '../../../widgets/histogram/histogram_types';
 import {buildNormalizedHistograms} from '../../../widgets/histogram/histogram_util';
 import {linkedTimeSelectionChanged, linkedTimeToggled} from '../../actions';
@@ -241,6 +244,10 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
   }
 
   onLinkedTimeToggled() {
-    this.store.dispatch(linkedTimeToggled());
+    this.store.dispatch(
+      linkedTimeToggled({
+        affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
+      })
+    );
   }
 }
diff --git a/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts
@@ -27,7 +27,10 @@ import {
 } from '../../../selectors';
 import {MatIconTestingModule} from '../../../testing/mat_icon_module';
 import {DataLoadState} from '../../../types/data';
-import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
+import {
+  TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
+} from '../../../widgets/card_fob/card_fob_types';
 import {
   HistogramData,
   HistogramMode,
@@ -554,7 +557,11 @@ describe('histogram card', () => {
         ).componentInstance;
         histogramWidget.onLinkedTimeToggled.emit();
 
-        expect(dispatchedActions).toEqual([linkedTimeToggled()]);
+        expect(dispatchedActions).toEqual([
+          linkedTimeToggled({
+            affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
+          }),
+        ]);
       });
     });
   });

diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts
@@ -27,6 +27,7 @@ import {DataLoadState} from '../../../types/data';
 import {
   TimeSelection,
   TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
 } from '../../../widgets/card_fob/card_fob_types';
 import {
   Formatter,
@@ -94,12 +95,14 @@ export class ScalarCardComponent<Downloader> {
 
   @Output() onFullSizeToggle = new EventEmitter<void>();
   @Output() onPinClicked = new EventEmitter<boolean>();
-  @Output() onLinkedTimeToggled = new EventEmitter();
+  @Output() onLinkedTimeToggled =
+    new EventEmitter<TimeSelectionToggleAffordance>();
   @Output() onLinkedTimeSelectionChanged = new EventEmitter<{
     timeSelection: TimeSelection;
     affordance: TimeSelectionAffordance;
   }>();
-  @Output() onStepSelectorToggled = new EventEmitter();
+  @Output() onStepSelectorToggled =
+    new EventEmitter<TimeSelectionToggleAffordance>();
   @Output() onStepSelectorTimeSelectionChanged = new EventEmitter<{
     timeSelection: TimeSelection;
     affordance: TimeSelectionAffordance;
@@ -308,9 +311,11 @@ export class ScalarCardComponent<Downloader> {
 
   onFobRemoved() {
     if (this.linkedTimeSelection !== null) {
-      this.onLinkedTimeToggled.emit();
+      this.onLinkedTimeToggled.emit(TimeSelectionToggleAffordance.FOB_DESELECT);
     } else {
-      this.onStepSelectorToggled.emit();
+      this.onStepSelectorToggled.emit(
+        TimeSelectionToggleAffordance.FOB_DESELECT
+      );
     }
   }
 }
diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts
@@ -54,6 +54,7 @@ import {DataLoadState} from '../../../types/data';
 import {
   TimeSelection,
   TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
 } from '../../../widgets/card_fob/card_fob_types';
 import {classicSmoothing} from '../../../widgets/line_chart_v2/data_transformer';
 import {ScaleType} from '../../../widgets/line_chart_v2/types';
@@ -152,8 +153,8 @@ function areSeriesEqual(
       (onStepSelectorTimeSelectionChanged)="
         onStepSelectorTimeSelectionChanged($event)
       "
-      (onLinkedTimeToggled)="onLinkedTimeToggled()"
-      (onStepSelectorToggled)="onStepSelectorToggled()"
+      (onLinkedTimeToggled)="onLinkedTimeToggled($event)"
+      (onStepSelectorToggled)="onStepSelectorToggled($event)"
     ></scalar-card-component>
   `,
   styles: [
@@ -602,11 +603,11 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
     );
   }
 
-  onLinkedTimeToggled() {
-    this.store.dispatch(linkedTimeToggled());
+  onLinkedTimeToggled(affordance: TimeSelectionToggleAffordance) {
+    this.store.dispatch(linkedTimeToggled({affordance}));
   }
 
-  onStepSelectorToggled() {
-    this.store.dispatch(stepSelectorToggled());
+  onStepSelectorToggled(affordance: TimeSelectionToggleAffordance) {
+    this.store.dispatch(stepSelectorToggled({affordance}));
   }
 }
diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts
@@ -51,7 +51,10 @@ import {
   Fob,
 } from '../../../widgets/card_fob/card_fob_controller_component';
 import {CardFobModule} from '../../../widgets/card_fob/card_fob_module';
-import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
+import {
+  TimeSelectionAffordance,
+  TimeSelectionToggleAffordance,
+} from '../../../widgets/card_fob/card_fob_types';
 import {DataTableComponent} from '../../../widgets/data_table/data_table_component';
 import {DataTableModule} from '../../../widgets/data_table/data_table_module';
 import {ExperimentAliasModule} from '../../../widgets/experiment_alias/experiment_alias_module';
@@ -2296,7 +2299,11 @@ describe('scalar card', () => {
         ).componentInstance;
         fobComponent.fobRemoved.emit();
 
-        expect(dispatchedActions).toEqual([linkedTimeToggled()]);
+        expect(dispatchedActions).toEqual([
+          linkedTimeToggled({
+            affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
+          }),
+        ]);
       }));
     });
 
@@ -2786,7 +2793,11 @@ describe('scalar card', () => {
         ).componentInstance;
         fobComponent.fobRemoved.emit();
 
-        expect(dispatchedActions).toEqual([stepSelectorToggled()]);
+        expect(dispatchedActions).toEqual([
+          stepSelectorToggled({
+            affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
+          }),
+        ]);
       }));
     });
 

diff --git a/tensorboard/webapp/metrics/views/right_pane/BUILD b/tensorboard/webapp/metrics/views/right_pane/BUILD
@@ -32,6 +32,7 @@ tf_ng_module(
         "//tensorboard/webapp/feature_flag",
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/actions",
+        "//tensorboard/webapp/widgets/card_fob:types",
         "//tensorboard/webapp/widgets/dropdown",
         "//tensorboard/webapp/widgets/range_input",
         "@npm//@angular/common",
@@ -61,6 +62,7 @@ tf_ts_library(
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/actions",
         "//tensorboard/webapp/metrics/store",
+        "//tensorboard/webapp/widgets/card_fob:types",
         "//tensorboard/webapp/widgets/dropdown",
         "@npm//@angular/core",
         "@npm//@angular/platform-browser",

diff --git a/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts b/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts
@@ -29,6 +29,7 @@ import {Store} from '@ngrx/store';
 import {MockStore, provideMockStore} from '@ngrx/store/testing';
 import {State} from '../../../app_state';
 import * as selectors from '../../../selectors';
+import {TimeSelectionToggleAffordance} from '../../../widgets/card_fob/card_fob_types';
 import {DropdownModule} from '../../../widgets/dropdown/dropdown_module';
 import * as actions from '../../actions';
 import {HistogramMode, TooltipSort, XAxisType} from '../../types';
@@ -432,7 +433,9 @@ describe('metrics right_pane', () => {
           enabled.nativeElement.click();
 
           expect(dispatchSpy).toHaveBeenCalledOnceWith(
-            actions.linkedTimeToggled()
+            actions.linkedTimeToggled({
+              affordance: TimeSelectionToggleAffordance.CHECK_BOX,
+            })
           );
 
           store.overrideSelector(selectors.getMetricsLinkedTimeEnabled, true);
@@ -598,7 +601,11 @@ describe('metrics right_pane', () => {
 
         select(fixture, '.scalars-step-selector input').nativeElement.click();
 
-        expect(dispatchSpy).toHaveBeenCalledWith(actions.stepSelectorToggled());
+        expect(dispatchSpy).toHaveBeenCalledWith(
+          actions.stepSelectorToggled({
+            affordance: TimeSelectionToggleAffordance.CHECK_BOX,
+          })
+        );
       });
     });
   });

diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts
@@ -18,6 +18,7 @@ import {Observable} from 'rxjs';
 import {filter, map, take, withLatestFrom} from 'rxjs/operators';
 import {State} from '../../../app_state';
 import * as selectors from '../../../selectors';
+import {TimeSelectionToggleAffordance} from '../../../widgets/card_fob/card_fob_types';
 import {
   linkedTimeSelectionChanged,
   linkedTimeToggled,
@@ -200,11 +201,15 @@ export class SettingsViewContainer {
   }
 
   onLinkedTimeToggled() {
-    this.store.dispatch(linkedTimeToggled());
+    this.store.dispatch(
+      linkedTimeToggled({affordance: TimeSelectionToggleAffordance.CHECK_BOX})
+    );
   }
 
   onStepSelectorToggled() {
-    this.store.dispatch(stepSelectorToggled());
+    this.store.dispatch(
+      stepSelectorToggled({affordance: TimeSelectionToggleAffordance.CHECK_BOX})
+    );
   }
 
   onLinkedTimeSelectionChanged(newValue: TimeSelection) {

diff --git a/tensorboard/webapp/widgets/card_fob/card_fob_types.ts b/tensorboard/webapp/widgets/card_fob/card_fob_types.ts
@@ -21,7 +21,8 @@ export interface TimeSelection {
 }
 
 /**
- * The affordance supported to update the time selection.
+ * The affordance supported to update the time selection in  step selector and linked time.
+ * Only used for internal analytics.
  */
 export enum TimeSelectionAffordance {
   NONE,
@@ -39,6 +40,18 @@ export enum TimeSelectionAffordance {
   SETTINGS_SLIDER,
 }
 
+/**
+ * The affordance supported to toggle step selector and linked time.
+ * Only used for internal analytics.
+ */
+export enum TimeSelectionToggleAffordance {
+  NONE,
+  // Clicking cross sign in fob.
+  FOB_DESELECT,
+  // Clicking check box in settings pane.
+  CHECK_BOX,
+}
+
 /**
  * The direction of the axis used to control the fob movements.
  */