Skip to content

Commit af8acdc

Browse files
authored
Linked Time: Render end fob based on the status of rangeSelectionEnabled (#6033)
## Motivation for features / changes We currently have two sources of truth for determining if range selection is enabled, the existence of the timeSelection end step AND the state variable `rangeSelectionEnabled`. This means that when range selection is disabled we need to clear the end step. In a future PR I will stop the end step from being removed when range selection is disabled. ## Screenshots of UI changes None * Detailed steps to verify changes work correctly (as executed by you) 1) Start tensorboard 2) Go to http://localhost:6006?enableRangeSelection 3) Enable range selection 4) Ensure a second fob appears on the scalar card 5) Disable range selection 6) Assert the end fob is removed.
1 parent 9fb5e24 commit af8acdc

File tree

8 files changed

+52
-1
lines changed

8 files changed

+52
-1
lines changed

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@
194194
[minMaxHorizontalViewExtend]="viewExtent.x"
195195
[minMaxStep]="minMaxStep"
196196
[axisSize]="domDim.width"
197+
[rangeSelectionEnabled]="rangeSelectionEnabled"
197198
[isProspectiveFobFeatureEnabled]="isProspectiveFobFeatureEnabled"
198199
(onTimeSelectionChanged)="onTimeSelectionChanged.emit($event)"
199200
(onTimeSelectionToggled)="onFobRemoved()"

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export class ScalarCardComponent<Downloader> {
9292
@Input() forceSvg!: boolean;
9393
@Input() linkedTimeSelection!: TimeSelectionView | null;
9494
@Input() stepOrLinkedTimeSelection!: TimeSelection | null;
95+
@Input() rangeSelectionEnabled: boolean = false;
9596
@Input() isProspectiveFobFeatureEnabled: Boolean = false;
9697
@Input() minMaxStep!: MinMaxStep;
9798
@Input() dataHeaders!: ColumnHeaders[];

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ function areSeriesEqual(
157157
[useDarkMode]="useDarkMode$ | async"
158158
[linkedTimeSelection]="linkedTimeSelection$ | async"
159159
[stepOrLinkedTimeSelection]="stepOrLinkedTimeSelection$ | async"
160+
[rangeSelectionEnabled]="rangeSelectionEnabled$ | async"
160161
[isProspectiveFobFeatureEnabled]="isProspectiveFobFeatureEnabled$ | async"
161162
[forceSvg]="forceSvg$ | async"
162163
[minMaxStep]="minMaxSteps$ | async"
@@ -223,6 +224,9 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
223224
});
224225
readonly stepSelectorTimeSelection$ =
225226
new BehaviorSubject<TimeSelection | null>(null);
227+
readonly rangeSelectionEnabled$ = this.store.select(
228+
getMetricsRangeSelectionEnabled
229+
);
226230
readonly useDarkMode$ = this.store.select(getDarkModeEnabled);
227231
readonly ignoreOutliers$ = this.store.select(getMetricsIgnoreOutliers);
228232
readonly tooltipSort$ = this.store.select(getMetricsTooltipSort);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {MinMaxStep} from './scalar_card_types';
3232
<card-fob-controller
3333
[axisDirection]="axisDirection"
3434
[timeSelection]="timeSelection"
35+
[rangeSelectionEnabled]="rangeSelectionEnabled"
3536
[startStepAxisPosition]="getAxisPositionFromStartStep()"
3637
[endStepAxisPosition]="getAxisPositionFromEndStep()"
3738
[prospectiveStepAxisPosition]="getAxisPositionFromProspectiveStep()"
@@ -55,6 +56,7 @@ export class ScalarCardFobController {
5556
@Input() minMaxHorizontalViewExtend!: [number, number];
5657
@Input() minMaxStep!: MinMaxStep;
5758
@Input() axisSize!: number;
59+
@Input() rangeSelectionEnabled: boolean = false;
5860
@Input() isProspectiveFobFeatureEnabled: Boolean = false;
5961

6062
@Output() onTimeSelectionChanged = new EventEmitter<{

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3299,6 +3299,19 @@ describe('scalar card', () => {
32993299
).toBeFalsy();
33003300
}));
33013301

3302+
it('Does not render end fob when range selection is disabled', fakeAsync(() => {
3303+
store.overrideSelector(
3304+
selectors.getMetricsRangeSelectionEnabled,
3305+
false
3306+
);
3307+
const fixture = createComponent('card1');
3308+
fixture.detectChanges();
3309+
3310+
expect(
3311+
fixture.debugElement.queryAll(By.directive(CardFobComponent)).length
3312+
).toEqual(1);
3313+
}));
3314+
33023315
it('dispatches timeSelectionChanged actions when fob is dragged while linked time is enabled', fakeAsync(() => {
33033316
store.overrideSelector(getMetricsLinkedTimeEnabled, true);
33043317
store.overrideSelector(getMetricsLinkedTimeSelection, {

tensorboard/webapp/widgets/card_fob/card_fob_controller_component.ng.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
</div>
5858
<div
5959
#endFobWrapper
60-
*ngIf="timeSelection && timeSelection.end"
60+
*ngIf="timeSelection && timeSelection.end && rangeSelectionEnabled"
6161
class="time-fob-wrapper"
6262
[style.transform]="getCssTranslatePxForEndFob()"
6363
>

tensorboard/webapp/widgets/card_fob/card_fob_controller_component.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export class CardFobControllerComponent {
5555
readonly prospectiveFobWrapper!: ElementRef;
5656
@Input() axisDirection!: AxisDirection;
5757
@Input() timeSelection?: TimeSelection;
58+
@Input() rangeSelectionEnabled: boolean = false;
5859
@Input() cardFobHelper!: CardFobGetStepFromPositionHelper;
5960
@Input() startStepAxisPosition!: number;
6061
@Input() endStepAxisPosition!: number | null;

tensorboard/webapp/widgets/card_fob/card_fob_controller_test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
#FobController
3434
[axisDirection]="axisDirection"
3535
[timeSelection]="timeSelection"
36+
[rangeSelectionEnabled]="rangeSelectionEnabled"
3637
[startStepAxisPosition]="getAxisPositionFromStartStep()"
3738
[endStepAxisPosition]="getAxisPositionFromEndStep()"
3839
[prospectiveStepAxisPosition]="getAxisPositionFromProspectiveStep()"
@@ -54,6 +55,7 @@ class TestableComponent {
5455

5556
@Input() axisDirection!: AxisDirection;
5657
@Input() timeSelection!: TimeSelection;
58+
@Input() rangeSelectionEnabled: boolean = false;
5759
@Input() cardFobHelper!: CardFobGetStepFromPositionHelper;
5860
@Input() showExtendedLine?: Boolean;
5961
@Input() highestStep!: number;
@@ -94,6 +96,7 @@ describe('card_fob_controller', () => {
9496
function createComponent(input: {
9597
axisDirection?: AxisDirection;
9698
timeSelection: TimeSelection;
99+
enableRangeSelection?: boolean;
97100
showExtendedLine?: Boolean;
98101
steps?: number[];
99102
isProspectiveFobFeatureEnabled?: Boolean;
@@ -150,6 +153,9 @@ describe('card_fob_controller', () => {
150153
input.axisDirection ?? AxisDirection.VERTICAL;
151154

152155
fixture.componentInstance.timeSelection = input.timeSelection;
156+
fixture.componentInstance.rangeSelectionEnabled = Boolean(
157+
input.enableRangeSelection
158+
);
153159

154160
fixture.componentInstance.showExtendedLine =
155161
input.showExtendedLine ?? false;
@@ -183,6 +189,7 @@ describe('card_fob_controller', () => {
183189
it('sets fob position based on time selection', () => {
184190
const fixture = createComponent({
185191
timeSelection: {start: {step: 2}, end: {step: 5}},
192+
enableRangeSelection: true,
186193
axisDirection: AxisDirection.HORIZONTAL,
187194
});
188195
fixture.detectChanges();
@@ -253,6 +260,7 @@ describe('card_fob_controller', () => {
253260
it('swaps fobs being dragged when start > end', () => {
254261
const fixture = createComponent({
255262
timeSelection: {start: {step: 1}, end: {step: 2}},
263+
enableRangeSelection: true,
256264
});
257265
fixture.detectChanges();
258266
const fobController = fixture.componentInstance.fobController;
@@ -286,6 +294,7 @@ describe('card_fob_controller', () => {
286294
it('swaps fobs being dragged when end < start', () => {
287295
const fixture = createComponent({
288296
timeSelection: {start: {step: 2}, end: {step: 3}},
297+
enableRangeSelection: true,
289298
});
290299
fixture.detectChanges();
291300
const fobController = fixture.componentInstance.fobController;
@@ -319,6 +328,7 @@ describe('card_fob_controller', () => {
319328
it('does not swaps fobs when start === end', () => {
320329
const fixture = createComponent({
321330
timeSelection: {start: {step: 2}, end: {step: 3}},
331+
enableRangeSelection: true,
322332
});
323333
fixture.detectChanges();
324334
const fobController = fixture.componentInstance.fobController;
@@ -353,6 +363,7 @@ describe('card_fob_controller', () => {
353363
it('does not fire event when time selection does not change', () => {
354364
const fixture = createComponent({
355365
timeSelection: {start: {step: 2}, end: {step: 3}},
366+
enableRangeSelection: true,
356367
});
357368
fixture.detectChanges();
358369
const fobController = fixture.componentInstance.fobController;
@@ -544,6 +555,7 @@ describe('card_fob_controller', () => {
544555
it('end fob moves to the mouse when mouse is dragging up and mouse is above the fob', () => {
545556
const fixture = createComponent({
546557
timeSelection: {start: {step: 1}, end: {step: 1}},
558+
enableRangeSelection: true,
547559
});
548560
fixture.detectChanges();
549561
const fobController = fixture.componentInstance.fobController;
@@ -582,6 +594,7 @@ describe('card_fob_controller', () => {
582594
it('end fob moves to the mouse when mouse is dragging down and mouse is below the fob', () => {
583595
const fixture = createComponent({
584596
timeSelection: {start: {step: 1}, end: {step: 4}},
597+
enableRangeSelection: true,
585598
});
586599
fixture.detectChanges();
587600
const fobController = fixture.componentInstance.fobController;
@@ -619,6 +632,7 @@ describe('card_fob_controller', () => {
619632
it('end fob does not move when mouse is dragging down but, mouse is above the fob', () => {
620633
const fixture = createComponent({
621634
timeSelection: {start: {step: 1}, end: {step: 2}},
635+
enableRangeSelection: true,
622636
});
623637
fixture.detectChanges();
624638
const fobController = fixture.componentInstance.fobController;
@@ -649,6 +663,7 @@ describe('card_fob_controller', () => {
649663
it('end fob does not move when mouse is dragging up but, mouse is below the fob', () => {
650664
const fixture = createComponent({
651665
timeSelection: {start: {step: 1}, end: {step: 3}},
666+
enableRangeSelection: true,
652667
});
653668
fixture.detectChanges();
654669
const fobController = fixture.componentInstance.fobController;
@@ -855,6 +870,7 @@ describe('card_fob_controller', () => {
855870
it('end fob moves to the mouse when mouse is dragging left and mouse is left of the fob', () => {
856871
const fixture = createComponent({
857872
timeSelection: {start: {step: 1}, end: {step: 1}},
873+
enableRangeSelection: true,
858874
axisDirection: AxisDirection.HORIZONTAL,
859875
});
860876
fixture.detectChanges();
@@ -894,6 +910,7 @@ describe('card_fob_controller', () => {
894910
it('end fob moves to the mouse when mouse is dragging right and mouse is right of the fob', () => {
895911
const fixture = createComponent({
896912
timeSelection: {start: {step: 1}, end: {step: 4}},
913+
enableRangeSelection: true,
897914
axisDirection: AxisDirection.HORIZONTAL,
898915
});
899916
fixture.detectChanges();
@@ -932,6 +949,7 @@ describe('card_fob_controller', () => {
932949
it('end fob does not move when mouse is dragging right but, mouse is left of the fob', () => {
933950
const fixture = createComponent({
934951
timeSelection: {start: {step: 1}, end: {step: 2}},
952+
enableRangeSelection: true,
935953
axisDirection: AxisDirection.HORIZONTAL,
936954
});
937955
fixture.detectChanges();
@@ -963,6 +981,7 @@ describe('card_fob_controller', () => {
963981
it('end fob does not move when mouse is dragging left but, mouse is right of the fob', () => {
964982
const fixture = createComponent({
965983
timeSelection: {start: {step: 1}, end: {step: 3}},
984+
enableRangeSelection: true,
966985
axisDirection: AxisDirection.HORIZONTAL,
967986
});
968987
fixture.detectChanges();
@@ -1008,6 +1027,7 @@ describe('card_fob_controller', () => {
10081027
it('renders two lines on range selection', () => {
10091028
const fixture = createComponent({
10101029
timeSelection: {start: {step: 1}, end: {step: 4}},
1030+
enableRangeSelection: true,
10111031
showExtendedLine: true,
10121032
});
10131033
fixture.detectChanges();
@@ -1021,6 +1041,7 @@ describe('card_fob_controller', () => {
10211041
it('clicks and drags the line to change selected step in horizontal axis', () => {
10221042
const fixture = createComponent({
10231043
timeSelection: {start: {step: 1}, end: {step: 3}},
1044+
enableRangeSelection: true,
10241045
axisDirection: AxisDirection.HORIZONTAL,
10251046
showExtendedLine: true,
10261047
});
@@ -1111,6 +1132,7 @@ describe('card_fob_controller', () => {
11111132
it('range selection start fob step typed which is less than end fob step', () => {
11121133
const fixture = createComponent({
11131134
timeSelection: {start: {step: 1}, end: {step: 4}},
1135+
enableRangeSelection: true,
11141136
});
11151137
fixture.detectChanges();
11161138
const fobController = fixture.componentInstance.fobController;
@@ -1128,6 +1150,7 @@ describe('card_fob_controller', () => {
11281150
it('range selection end fob step typed which is greater than start fob step', () => {
11291151
const fixture = createComponent({
11301152
timeSelection: {start: {step: 1}, end: {step: 4}},
1153+
enableRangeSelection: true,
11311154
});
11321155
fixture.detectChanges();
11331156
const fobController = fixture.componentInstance.fobController;
@@ -1145,6 +1168,7 @@ describe('card_fob_controller', () => {
11451168
it('range selection swaps when start step is typed in which is greater than end step', () => {
11461169
const fixture = createComponent({
11471170
timeSelection: {start: {step: 1}, end: {step: 2}},
1171+
enableRangeSelection: true,
11481172
});
11491173
fixture.detectChanges();
11501174
const fobController = fixture.componentInstance.fobController;
@@ -1162,6 +1186,7 @@ describe('card_fob_controller', () => {
11621186
it('range selection swaps when end step is typed in which is less than start step', () => {
11631187
const fixture = createComponent({
11641188
timeSelection: {start: {step: 3}, end: {step: 4}},
1189+
enableRangeSelection: true,
11651190
});
11661191
fixture.detectChanges();
11671192
const fobController = fixture.componentInstance.fobController;
@@ -1179,6 +1204,7 @@ describe('card_fob_controller', () => {
11791204
it('properly handles a 0 step', () => {
11801205
const fixture = createComponent({
11811206
timeSelection: {start: {step: 3}, end: {step: 4}},
1207+
enableRangeSelection: true,
11821208
});
11831209
fixture.detectChanges();
11841210
const fobController = fixture.componentInstance.fobController;
@@ -1220,6 +1246,7 @@ describe('card_fob_controller', () => {
12201246
it('changing end fob input modifies end step', () => {
12211247
const fixture = createComponent({
12221248
timeSelection: {start: {step: 1}, end: {step: 3}},
1249+
enableRangeSelection: true,
12231250
});
12241251
fixture.detectChanges();
12251252

@@ -1263,6 +1290,7 @@ describe('card_fob_controller', () => {
12631290
it('fires onTimeSelectionChanged to remove end fob when end fob is deselected', () => {
12641291
const fixture = createComponent({
12651292
timeSelection: {start: {step: 1}, end: {step: 3}},
1293+
enableRangeSelection: true,
12661294
});
12671295
fixture.detectChanges();
12681296

@@ -1284,6 +1312,7 @@ describe('card_fob_controller', () => {
12841312
it('fires onTimeSelectionChanged to change the start fob step to the current end fob step and the end fob step to null when start fob is deselected in a range selection', () => {
12851313
const fixture = createComponent({
12861314
timeSelection: {start: {step: 1}, end: {step: 3}},
1315+
enableRangeSelection: true,
12871316
});
12881317
fixture.detectChanges();
12891318

0 commit comments

Comments
 (0)