From db79ff2e3403b45790d4712dfc8df6dfcc109100 Mon Sep 17 00:00:00 2001 From: japie Date: Mon, 6 Jun 2022 13:12:44 -0700 Subject: [PATCH] step selector: add control to settings panel behind feature flag (#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). --- .../views/right_pane/right_pane_test.ts | 27 +++++++++++++++++++ .../settings_view_component.ng.html | 11 ++++++++ .../right_pane/settings_view_component.ts | 3 +++ .../right_pane/settings_view_container.ts | 8 ++++++ 4 files changed, 49 insertions(+) diff --git a/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts b/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts index 3becc7ae04..f0f8018e3f 100644 --- a/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts +++ b/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts @@ -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); @@ -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); @@ -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'); + }); + }); }); }); diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html index c679bc4492..64cc701b8d 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ng.html @@ -127,6 +127,17 @@

Scalars

> +
+ Step Selector +
+
(); @Output() selectTimeChanged = new EventEmitter(); + @Output() stepSelectorEnableToggled = new EventEmitter(); + @Input() isImageSupportEnabled!: boolean; readonly TooltipSortDropdownOptions: DropdownOption[] = [ diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts index d2fb085154..659570d8ef 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_container.ts @@ -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()" > `, @@ -87,6 +89,8 @@ export class SettingsViewContainer { readonly isLinkedTimeFeatureEnabled$: Observable = this.store.select( selectors.getIsLinkedTimeEnabled ); + readonly isScalarStepSelectorEnabled$: Observable = + this.store.select(selectors.getIsDataTableEnabled); readonly selectTimeEnabled$: Observable = this.store.select( selectors.getMetricsSelectTimeEnabled ); @@ -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({