Skip to content

Commit d9c2cc1

Browse files
authored
Linked Time: Make timeSelection optional in fob controllers (#6005)
* Motivation for features / changes We would like to be able to enable step selection using a prospective fob. The prospective fob is currently implemented withing the fob_controller_component which has a required `timeSelection` parameter. * 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?enableDataTable=true&enableProspectiveFob=true](http://localhost:6006?enableDataTable=true&enableProspectiveFob=true) 3) View a scalar card 4) Assert that no fobs are shown 5) Enable step selection 6) Assert that a single fob is shown
1 parent 6be2464 commit d9c2cc1

File tree

4 files changed

+42
-15
lines changed

4 files changed

+42
-15
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import {MinMaxStep} from './scalar_card_types';
4949
changeDetection: ChangeDetectionStrategy.OnPush,
5050
})
5151
export class ScalarCardFobController {
52-
@Input() timeSelection!: TimeSelection;
52+
@Input() timeSelection?: TimeSelection;
5353
@Input() scale!: Scale;
5454
@Input() minMaxHorizontalViewExtend!: [number, number];
5555
@Input() minMaxStep!: MinMaxStep;
@@ -71,6 +71,9 @@ export class ScalarCardFobController {
7171
prospectiveStep: number | null = null;
7272

7373
getAxisPositionFromStartStep() {
74+
if (!this.timeSelection) {
75+
return '';
76+
}
7477
return this.scale.forward(
7578
this.minMaxHorizontalViewExtend,
7679
[0, this.axisSize],
@@ -79,13 +82,13 @@ export class ScalarCardFobController {
7982
}
8083

8184
getAxisPositionFromEndStep() {
82-
if (this.timeSelection.end === null) {
85+
if (!this.timeSelection?.end) {
8386
return null;
8487
}
8588
return this.scale.forward(
8689
this.minMaxHorizontalViewExtend,
8790
[0, this.axisSize],
88-
this.timeSelection.end.step
91+
this.timeSelection?.end.step ?? this.minMaxStep.maxStep
8992
);
9093
}
9194

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2376,6 +2376,26 @@ describe('scalar card', () => {
23762376
}),
23772377
]);
23782378
}));
2379+
2380+
// TODO(rileyajones) refactor this test with #6006
2381+
it('does not render fobs when no timeSelection is provided', fakeAsync(() => {
2382+
store.overrideSelector(getMetricsLinkedTimeSelection, {
2383+
start: {step: 20},
2384+
end: null,
2385+
});
2386+
const fixture = createComponent('card1');
2387+
const scalarCardFobController = fixture.debugElement.query(
2388+
By.directive(ScalarCardFobController)
2389+
).componentInstance;
2390+
delete scalarCardFobController.timeSelection;
2391+
fixture.detectChanges();
2392+
const fobController = fixture.debugElement.query(
2393+
By.directive(CardFobComponent)
2394+
).componentInstance;
2395+
2396+
expect(fobController.startFobWrapper).toBeUndefined();
2397+
expect(fobController.endFobWrapper).toBeUndefined();
2398+
}));
23792399
});
23802400

23812401
describe('scalar card data table', () => {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#startFobWrapper
3232
class="time-fob-wrapper"
3333
[style.transform]="getCssTranslatePxForStartFob()"
34+
*ngIf="timeSelection"
3435
>
3536
<div
3637
*ngIf="showExtendedLine"
@@ -48,7 +49,7 @@
4849
</div>
4950
<div
5051
#endFobWrapper
51-
*ngIf="timeSelection.end"
52+
*ngIf="timeSelection && timeSelection.end"
5253
class="time-fob-wrapper"
5354
[style.transform]="getCssTranslatePxForEndFob()"
5455
>

tensorboard/webapp/widgets/card_fob/card_fob_controller_component.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export class CardFobControllerComponent {
5353
@ViewChild('prospectiveFobWrapper')
5454
readonly prospectiveFobWrapper!: ElementRef;
5555
@Input() axisDirection!: AxisDirection;
56-
@Input() timeSelection!: TimeSelection;
56+
@Input() timeSelection?: TimeSelection;
5757
@Input() cardFobHelper!: CardFobGetStepFromPositionHelper;
5858
@Input() startStepAxisPosition!: number;
5959
@Input() endStepAxisPosition!: number | null;
@@ -134,7 +134,7 @@ export class CardFobControllerComponent {
134134
document.removeEventListener('mousemove', this.mouseListener);
135135
document.removeEventListener('mouseup', this.stopListener);
136136
this.currentDraggingFob = Fob.NONE;
137-
if (this.hasFobMoved) {
137+
if (this.hasFobMoved && this.timeSelection) {
138138
this.onTimeSelectionChanged.emit({
139139
timeSelection: this.timeSelection,
140140
affordance: this.affordance,
@@ -149,7 +149,7 @@ export class CardFobControllerComponent {
149149
}
150150

151151
private shouldSwapFobs(newStep: number) {
152-
if (!this.timeSelection.end) {
152+
if (!this.timeSelection || !this.timeSelection.end) {
153153
return false;
154154
}
155155
if (this.currentDraggingFob === Fob.END) {
@@ -168,6 +168,9 @@ export class CardFobControllerComponent {
168168
): TimeSelection {
169169
const newTimeSelection = {...timeSelection};
170170

171+
if (!this.timeSelection) {
172+
return newTimeSelection;
173+
}
171174
// Single Selection
172175
if (!this.timeSelection.end) {
173176
newTimeSelection.start = {step: newStep};
@@ -211,7 +214,7 @@ export class CardFobControllerComponent {
211214
newStep = this.cardFobHelper.getStepLowerThanAxisPosition(mousePosition);
212215
}
213216

214-
if (newStep === null) {
217+
if (newStep === null || !this.timeSelection) {
215218
return;
216219
}
217220

@@ -267,8 +270,8 @@ export class CardFobControllerComponent {
267270

268271
getDraggingFobStep(): number {
269272
return this.currentDraggingFob !== Fob.END
270-
? this.timeSelection.start.step
271-
: this.timeSelection.end!.step;
273+
? this.timeSelection!.start.step
274+
: this.timeSelection!.end!.step;
272275
}
273276

274277
getMousePositionFromEvent(event: MouseEvent): number {
@@ -281,7 +284,7 @@ export class CardFobControllerComponent {
281284
// Types empty string in fob.
282285
if (step === null) {
283286
// Removes fob on range selection and sets step to minimum on single selection.
284-
if (this.timeSelection.end !== null) {
287+
if (this.timeSelection!.end !== null) {
285288
this.onFobRemoved(fob);
286289
} else {
287290
// TODO(jieweiwu): sets start step to minum.
@@ -290,7 +293,7 @@ export class CardFobControllerComponent {
290293
return;
291294
}
292295

293-
let newTimeSelection = {...this.timeSelection};
296+
let newTimeSelection = {...this.timeSelection!};
294297
if (fob === Fob.START) {
295298
newTimeSelection.start = {step};
296299
} else if (fob === Fob.END) {
@@ -328,16 +331,16 @@ export class CardFobControllerComponent {
328331
if (fob === Fob.END) {
329332
this.onTimeSelectionChanged.emit({
330333
affordance: TimeSelectionAffordance.FOB_REMOVED,
331-
timeSelection: {...this.timeSelection, end: null},
334+
timeSelection: {...this.timeSelection!, end: null},
332335
});
333336
return;
334337
}
335338

336-
if (this.timeSelection.end !== null) {
339+
if (this.timeSelection!.end !== null) {
337340
this.onTimeSelectionChanged.emit({
338341
affordance: TimeSelectionAffordance.FOB_REMOVED,
339342
timeSelection: {
340-
start: this.timeSelection.end,
343+
start: this.timeSelection!.end,
341344
end: null,
342345
},
343346
});

0 commit comments

Comments
 (0)