Skip to content

Commit 0f4d6e9

Browse files
authored
linked time: add affordance for toggling action (#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.
1 parent cf342b0 commit 0f4d6e9

File tree

11 files changed

+107
-36
lines changed

11 files changed

+107
-36
lines changed

tensorboard/webapp/metrics/actions/index.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ limitations under the License.
1414
==============================================================================*/
1515
import {createAction, props} from '@ngrx/store';
1616
import {ElementId} from '../../util/dom';
17-
import {TimeSelectionAffordance} from '../../widgets/card_fob/card_fob_types';
17+
import {
18+
TimeSelectionAffordance,
19+
TimeSelectionToggleAffordance,
20+
} from '../../widgets/card_fob/card_fob_types';
1821
import {
1922
TagMetadata,
2023
TimeSeriesRequest,
@@ -177,7 +180,7 @@ export const linkedTimeSelectionChanged = createAction(
177180
startStep: number;
178181
endStep: number | undefined;
179182
};
180-
// Affordance for analytics purpose. When no affordance is specified or is
183+
// Affordance for internal analytics purpose. When no affordance is specified or is
181184
// undefined we do not want to log an analytics event.
182185
affordance?: TimeSelectionAffordance | undefined;
183186
}>()
@@ -188,11 +191,21 @@ export const timeSelectionCleared = createAction(
188191
);
189192

190193
export const linkedTimeToggled = createAction(
191-
'[Metrics] Linked Time Enable Toggle'
194+
'[Metrics] Linked Time Enable Toggle',
195+
props<{
196+
// Affordance for internal analytics purpose. When no affordance is specified or is
197+
// undefined we do not want to log an analytics event.
198+
affordance?: TimeSelectionToggleAffordance;
199+
}>()
192200
);
193201

194202
export const stepSelectorToggled = createAction(
195-
'[Metrics] Time Selector Enable Toggle'
203+
'[Metrics] Time Selector Enable Toggle',
204+
props<{
205+
// Affordance for internal analytics purpose. When no affordance is specified or is
206+
// undefined we do not want to log an analytics event.
207+
affordance?: TimeSelectionToggleAffordance;
208+
}>()
196209
);
197210

198211
/**

tensorboard/webapp/metrics/store/metrics_reducers_test.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2737,10 +2737,10 @@ describe('metrics reducers', () => {
27372737
linkedTimeEnabled: false,
27382738
});
27392739

2740-
const state2 = reducers(state1, actions.linkedTimeToggled());
2740+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
27412741
expect(state2.linkedTimeEnabled).toBe(true);
27422742

2743-
const state3 = reducers(state2, actions.linkedTimeToggled());
2743+
const state3 = reducers(state2, actions.linkedTimeToggled({}));
27442744
expect(state3.linkedTimeEnabled).toBe(false);
27452745
});
27462746

@@ -2770,7 +2770,7 @@ describe('metrics reducers', () => {
27702770
cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
27712771
});
27722772

2773-
const state2 = reducers(state1, actions.linkedTimeToggled());
2773+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
27742774

27752775
expect(state2.cardStepIndex).toEqual({
27762776
[imageCardId]: buildStepIndexMetadata({index: 0}),
@@ -2802,7 +2802,7 @@ describe('metrics reducers', () => {
28022802
cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
28032803
});
28042804

2805-
const state2 = reducers(state1, actions.linkedTimeToggled());
2805+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
28062806
expect(state2.cardStepIndex).toEqual({
28072807
[imageCardId]: buildStepIndexMetadata({index: 1}),
28082808
});
@@ -2840,7 +2840,7 @@ describe('metrics reducers', () => {
28402840
cardStepIndex: {[imageCardId]: buildStepIndexMetadata({index: 2})},
28412841
});
28422842

2843-
const state2 = reducers(state1, actions.linkedTimeToggled());
2843+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
28442844
expect(state2.cardStepIndex).toEqual({
28452845
[imageCardId]: buildStepIndexMetadata({index: 2}),
28462846
});
@@ -2851,7 +2851,7 @@ describe('metrics reducers', () => {
28512851
stepMinMax: {min: Infinity, max: -Infinity},
28522852
});
28532853

2854-
const state2 = reducers(state1, actions.linkedTimeToggled());
2854+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
28552855

28562856
expect(state2.linkedTimeSelection).toEqual({
28572857
start: {step: 0},
@@ -2864,7 +2864,7 @@ describe('metrics reducers', () => {
28642864
stepMinMax: {min: 10, max: 100},
28652865
});
28662866

2867-
const state2 = reducers(state1, actions.linkedTimeToggled());
2867+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
28682868

28692869
expect(state2.linkedTimeSelection).toEqual({
28702870
start: {step: 10},
@@ -2877,7 +2877,7 @@ describe('metrics reducers', () => {
28772877
linkedTimeSelection: {start: {step: 20}, end: null},
28782878
});
28792879

2880-
const state2 = reducers(state1, actions.linkedTimeToggled());
2880+
const state2 = reducers(state1, actions.linkedTimeToggled({}));
28812881
expect(state2.linkedTimeSelection).toEqual({
28822882
start: {step: 20},
28832883
end: null,
@@ -2892,10 +2892,10 @@ describe('metrics reducers', () => {
28922892
stepSelectorEnabled: false,
28932893
});
28942894

2895-
const state2 = reducers(state1, actions.stepSelectorToggled());
2895+
const state2 = reducers(state1, actions.stepSelectorToggled({}));
28962896
expect(state2.stepSelectorEnabled).toBe(true);
28972897

2898-
const state3 = reducers(state2, actions.stepSelectorToggled());
2898+
const state3 = reducers(state2, actions.stepSelectorToggled({}));
28992899
expect(state3.stepSelectorEnabled).toBe(false);
29002900
});
29012901
});

tensorboard/webapp/metrics/views/card_renderer/histogram_card_container.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ import {filter, map} from 'rxjs/operators';
2626
import {State} from '../../../app_state';
2727
import {DataLoadState} from '../../../types/data';
2828
import {RunColorScale} from '../../../types/ui';
29-
import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
29+
import {
30+
TimeSelectionAffordance,
31+
TimeSelectionToggleAffordance,
32+
} from '../../../widgets/card_fob/card_fob_types';
3033
import {HistogramDatum} from '../../../widgets/histogram/histogram_types';
3134
import {buildNormalizedHistograms} from '../../../widgets/histogram/histogram_util';
3235
import {linkedTimeSelectionChanged, linkedTimeToggled} from '../../actions';
@@ -241,6 +244,10 @@ export class HistogramCardContainer implements CardRenderer, OnInit {
241244
}
242245

243246
onLinkedTimeToggled() {
244-
this.store.dispatch(linkedTimeToggled());
247+
this.store.dispatch(
248+
linkedTimeToggled({
249+
affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
250+
})
251+
);
245252
}
246253
}

tensorboard/webapp/metrics/views/card_renderer/histogram_card_test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ import {
2727
} from '../../../selectors';
2828
import {MatIconTestingModule} from '../../../testing/mat_icon_module';
2929
import {DataLoadState} from '../../../types/data';
30-
import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
30+
import {
31+
TimeSelectionAffordance,
32+
TimeSelectionToggleAffordance,
33+
} from '../../../widgets/card_fob/card_fob_types';
3134
import {
3235
HistogramData,
3336
HistogramMode,
@@ -554,7 +557,11 @@ describe('histogram card', () => {
554557
).componentInstance;
555558
histogramWidget.onLinkedTimeToggled.emit();
556559

557-
expect(dispatchedActions).toEqual([linkedTimeToggled()]);
560+
expect(dispatchedActions).toEqual([
561+
linkedTimeToggled({
562+
affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
563+
}),
564+
]);
558565
});
559566
});
560567
});

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {DataLoadState} from '../../../types/data';
2727
import {
2828
TimeSelection,
2929
TimeSelectionAffordance,
30+
TimeSelectionToggleAffordance,
3031
} from '../../../widgets/card_fob/card_fob_types';
3132
import {
3233
Formatter,
@@ -94,12 +95,14 @@ export class ScalarCardComponent<Downloader> {
9495

9596
@Output() onFullSizeToggle = new EventEmitter<void>();
9697
@Output() onPinClicked = new EventEmitter<boolean>();
97-
@Output() onLinkedTimeToggled = new EventEmitter();
98+
@Output() onLinkedTimeToggled =
99+
new EventEmitter<TimeSelectionToggleAffordance>();
98100
@Output() onLinkedTimeSelectionChanged = new EventEmitter<{
99101
timeSelection: TimeSelection;
100102
affordance: TimeSelectionAffordance;
101103
}>();
102-
@Output() onStepSelectorToggled = new EventEmitter();
104+
@Output() onStepSelectorToggled =
105+
new EventEmitter<TimeSelectionToggleAffordance>();
103106
@Output() onStepSelectorTimeSelectionChanged = new EventEmitter<{
104107
timeSelection: TimeSelection;
105108
affordance: TimeSelectionAffordance;
@@ -308,9 +311,11 @@ export class ScalarCardComponent<Downloader> {
308311

309312
onFobRemoved() {
310313
if (this.linkedTimeSelection !== null) {
311-
this.onLinkedTimeToggled.emit();
314+
this.onLinkedTimeToggled.emit(TimeSelectionToggleAffordance.FOB_DESELECT);
312315
} else {
313-
this.onStepSelectorToggled.emit();
316+
this.onStepSelectorToggled.emit(
317+
TimeSelectionToggleAffordance.FOB_DESELECT
318+
);
314319
}
315320
}
316321
}

tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {DataLoadState} from '../../../types/data';
5454
import {
5555
TimeSelection,
5656
TimeSelectionAffordance,
57+
TimeSelectionToggleAffordance,
5758
} from '../../../widgets/card_fob/card_fob_types';
5859
import {classicSmoothing} from '../../../widgets/line_chart_v2/data_transformer';
5960
import {ScaleType} from '../../../widgets/line_chart_v2/types';
@@ -152,8 +153,8 @@ function areSeriesEqual(
152153
(onStepSelectorTimeSelectionChanged)="
153154
onStepSelectorTimeSelectionChanged($event)
154155
"
155-
(onLinkedTimeToggled)="onLinkedTimeToggled()"
156-
(onStepSelectorToggled)="onStepSelectorToggled()"
156+
(onLinkedTimeToggled)="onLinkedTimeToggled($event)"
157+
(onStepSelectorToggled)="onStepSelectorToggled($event)"
157158
></scalar-card-component>
158159
`,
159160
styles: [
@@ -602,11 +603,11 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
602603
);
603604
}
604605

605-
onLinkedTimeToggled() {
606-
this.store.dispatch(linkedTimeToggled());
606+
onLinkedTimeToggled(affordance: TimeSelectionToggleAffordance) {
607+
this.store.dispatch(linkedTimeToggled({affordance}));
607608
}
608609

609-
onStepSelectorToggled() {
610-
this.store.dispatch(stepSelectorToggled());
610+
onStepSelectorToggled(affordance: TimeSelectionToggleAffordance) {
611+
this.store.dispatch(stepSelectorToggled({affordance}));
611612
}
612613
}

tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ import {
5151
Fob,
5252
} from '../../../widgets/card_fob/card_fob_controller_component';
5353
import {CardFobModule} from '../../../widgets/card_fob/card_fob_module';
54-
import {TimeSelectionAffordance} from '../../../widgets/card_fob/card_fob_types';
54+
import {
55+
TimeSelectionAffordance,
56+
TimeSelectionToggleAffordance,
57+
} from '../../../widgets/card_fob/card_fob_types';
5558
import {DataTableComponent} from '../../../widgets/data_table/data_table_component';
5659
import {DataTableModule} from '../../../widgets/data_table/data_table_module';
5760
import {ExperimentAliasModule} from '../../../widgets/experiment_alias/experiment_alias_module';
@@ -2296,7 +2299,11 @@ describe('scalar card', () => {
22962299
).componentInstance;
22972300
fobComponent.fobRemoved.emit();
22982301

2299-
expect(dispatchedActions).toEqual([linkedTimeToggled()]);
2302+
expect(dispatchedActions).toEqual([
2303+
linkedTimeToggled({
2304+
affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
2305+
}),
2306+
]);
23002307
}));
23012308
});
23022309

@@ -2786,7 +2793,11 @@ describe('scalar card', () => {
27862793
).componentInstance;
27872794
fobComponent.fobRemoved.emit();
27882795

2789-
expect(dispatchedActions).toEqual([stepSelectorToggled()]);
2796+
expect(dispatchedActions).toEqual([
2797+
stepSelectorToggled({
2798+
affordance: TimeSelectionToggleAffordance.FOB_DESELECT,
2799+
}),
2800+
]);
27902801
}));
27912802
});
27922803

tensorboard/webapp/metrics/views/right_pane/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ tf_ng_module(
3232
"//tensorboard/webapp/feature_flag",
3333
"//tensorboard/webapp/metrics:types",
3434
"//tensorboard/webapp/metrics/actions",
35+
"//tensorboard/webapp/widgets/card_fob:types",
3536
"//tensorboard/webapp/widgets/dropdown",
3637
"//tensorboard/webapp/widgets/range_input",
3738
"@npm//@angular/common",
@@ -61,6 +62,7 @@ tf_ts_library(
6162
"//tensorboard/webapp/metrics:types",
6263
"//tensorboard/webapp/metrics/actions",
6364
"//tensorboard/webapp/metrics/store",
65+
"//tensorboard/webapp/widgets/card_fob:types",
6466
"//tensorboard/webapp/widgets/dropdown",
6567
"@npm//@angular/core",
6668
"@npm//@angular/platform-browser",

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {Store} from '@ngrx/store';
2929
import {MockStore, provideMockStore} from '@ngrx/store/testing';
3030
import {State} from '../../../app_state';
3131
import * as selectors from '../../../selectors';
32+
import {TimeSelectionToggleAffordance} from '../../../widgets/card_fob/card_fob_types';
3233
import {DropdownModule} from '../../../widgets/dropdown/dropdown_module';
3334
import * as actions from '../../actions';
3435
import {HistogramMode, TooltipSort, XAxisType} from '../../types';
@@ -432,7 +433,9 @@ describe('metrics right_pane', () => {
432433
enabled.nativeElement.click();
433434

434435
expect(dispatchSpy).toHaveBeenCalledOnceWith(
435-
actions.linkedTimeToggled()
436+
actions.linkedTimeToggled({
437+
affordance: TimeSelectionToggleAffordance.CHECK_BOX,
438+
})
436439
);
437440

438441
store.overrideSelector(selectors.getMetricsLinkedTimeEnabled, true);
@@ -598,7 +601,11 @@ describe('metrics right_pane', () => {
598601

599602
select(fixture, '.scalars-step-selector input').nativeElement.click();
600603

601-
expect(dispatchSpy).toHaveBeenCalledWith(actions.stepSelectorToggled());
604+
expect(dispatchSpy).toHaveBeenCalledWith(
605+
actions.stepSelectorToggled({
606+
affordance: TimeSelectionToggleAffordance.CHECK_BOX,
607+
})
608+
);
602609
});
603610
});
604611
});

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {Observable} from 'rxjs';
1818
import {filter, map, take, withLatestFrom} from 'rxjs/operators';
1919
import {State} from '../../../app_state';
2020
import * as selectors from '../../../selectors';
21+
import {TimeSelectionToggleAffordance} from '../../../widgets/card_fob/card_fob_types';
2122
import {
2223
linkedTimeSelectionChanged,
2324
linkedTimeToggled,
@@ -200,11 +201,15 @@ export class SettingsViewContainer {
200201
}
201202

202203
onLinkedTimeToggled() {
203-
this.store.dispatch(linkedTimeToggled());
204+
this.store.dispatch(
205+
linkedTimeToggled({affordance: TimeSelectionToggleAffordance.CHECK_BOX})
206+
);
204207
}
205208

206209
onStepSelectorToggled() {
207-
this.store.dispatch(stepSelectorToggled());
210+
this.store.dispatch(
211+
stepSelectorToggled({affordance: TimeSelectionToggleAffordance.CHECK_BOX})
212+
);
208213
}
209214

210215
onLinkedTimeSelectionChanged(newValue: TimeSelection) {

0 commit comments

Comments
 (0)