Skip to content

Commit

Permalink
step selector: add control to settings panel behind feature flag (ten…
Browse files Browse the repository at this point in the history
…sorflow#5734)

This is part of the work to separate the "sticky data table" (aka step selector) to its own feature. This PR adds the checkbox to enable/disable step selector. The checkbox is hided behind feature flag (enableScalarDataTable).
  • Loading branch information
japie1235813 authored and dna2github committed May 1, 2023
1 parent af6656b commit 0116816
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 0 deletions.
27 changes: 27 additions & 0 deletions tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('metrics right_pane', () => {
store.overrideSelector(selectors.getIsFeatureFlagsLoaded, true);
store.overrideSelector(selectors.getIsMetricsImageSupportEnabled, true);
store.overrideSelector(selectors.getIsLinkedTimeEnabled, false);
store.overrideSelector(selectors.getIsDataTableEnabled, false);
store.overrideSelector(selectors.getEnabledCardWidthSetting, false);
store.overrideSelector(selectors.getMetricsCardMinWidth, null);
store.overrideSelector(selectors.getMetricsSelectTimeEnabled, false);
Expand Down Expand Up @@ -385,6 +386,15 @@ describe('metrics right_pane', () => {
expect(el).toBeFalsy();
});

it('does not display step selector setting on step selector disabled', () => {
store.overrideSelector(selectors.getIsDataTableEnabled, false);
const fixture = TestBed.createComponent(SettingsViewContainer);
fixture.detectChanges();

const el = fixture.debugElement.query(By.css('.scalars-step-selector'));
expect(el).toBeFalsy();
});

describe('linked time feature enabled', () => {
beforeEach(() => {
store.overrideSelector(selectors.getIsLinkedTimeEnabled, true);
Expand Down Expand Up @@ -524,5 +534,22 @@ describe('metrics right_pane', () => {
});
});
});

describe('step selector feature enabled', () => {
beforeEach(() => {
store.overrideSelector(selectors.getIsDataTableEnabled, true);
});

it('renders', () => {
const fixture = TestBed.createComponent(SettingsViewContainer);
fixture.detectChanges();

expect(
select(fixture, '.scalars-step-selector input').attributes[
'aria-checked'
]
).toBe('false');
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ <h3 class="section-title">Scalars</h3>
>
</div>

<div
class="control-row scalars-step-selector"
*ngIf="isScalarStepSelectorEnabled"
>
<mat-checkbox
[checked]="stepSelectorEnabled"
(change)="stepSelectorEnableToggled.emit()"
>Step Selector</mat-checkbox
>
</div>

<div class="control-row scalars-partition-x">
<mat-checkbox
[checked]="scalarPartitionX"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class SettingsViewComponent {

@Input() isCardWidthSettingEnabled!: boolean;
@Input() isLinkedTimeFeatureEnabled!: boolean;
@Input() isScalarStepSelectorEnabled!: boolean;
@Input() selectTimeEnabled!: boolean;
@Input() useRangeSelectTime!: boolean;
@Input() selectedTime!: LinkedTime | null;
Expand All @@ -73,6 +74,8 @@ export class SettingsViewComponent {
@Output() selectTimeEnableToggled = new EventEmitter<void>();
@Output() selectTimeChanged = new EventEmitter<LinkedTime>();

@Output() stepSelectorEnableToggled = new EventEmitter<void>();

@Input() isImageSupportEnabled!: boolean;

readonly TooltipSortDropdownOptions: DropdownOption[] = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ import {HistogramMode, LinkedTime, TooltipSort, XAxisType} from '../../types';
[imageShowActualSize]="imageShowActualSize$ | async"
(imageShowActualSizeChanged)="onImageShowActualSizeChanged()"
[isLinkedTimeFeatureEnabled]="isLinkedTimeFeatureEnabled$ | async"
[isScalarStepSelectorEnabled]="isScalarStepSelectorEnabled$ | async"
[selectTimeEnabled]="selectTimeEnabled$ | async"
[selectedTime]="selectedTime$ | async"
[useRangeSelectTime]="useRangeSelectTime$ | async"
[stepMinMax]="stepMinMax$ | async"
(selectTimeEnableToggled)="onSelectTimeEnableToggled()"
(selectTimeChanged)="onSelectTimeChanged($event)"
(stepSelectorEnableToggled)="onStepSelectorEnableToggled()"
>
</metrics-dashboard-settings-component>
`,
Expand All @@ -87,6 +89,8 @@ export class SettingsViewContainer {
readonly isLinkedTimeFeatureEnabled$: Observable<boolean> = this.store.select(
selectors.getIsLinkedTimeEnabled
);
readonly isScalarStepSelectorEnabled$: Observable<boolean> =
this.store.select(selectors.getIsDataTableEnabled);
readonly selectTimeEnabled$: Observable<boolean> = this.store.select(
selectors.getMetricsSelectTimeEnabled
);
Expand Down Expand Up @@ -192,6 +196,10 @@ export class SettingsViewContainer {
this.store.dispatch(selectTimeEnableToggled());
}

onStepSelectorEnableToggled() {
// TODO(japie1235813): Dispatch toggled event to update ngrx state.
}

onSelectTimeChanged(newValue: LinkedTime) {
this.store.dispatch(
timeSelectionChanged({
Expand Down

0 comments on commit 0116816

Please sign in to comment.