Skip to content

Commit

Permalink
step selector: add control to settings panel behind feature flag (#5734)
Browse files Browse the repository at this point in the history
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 Jun 6, 2022
1 parent 0775fbc commit db79ff2
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 db79ff2

Please sign in to comment.