From 38ebe63bf423f9054a56329e82f16e0cef56b57f Mon Sep 17 00:00:00 2001 From: jameshollyer Date: Wed, 18 Oct 2023 17:47:59 +0000 Subject: [PATCH] MDCMigration: Migrate mat slider in settingsViewComponent --- tensorboard/webapp/angular/BUILD | 11 ++++- .../webapp/metrics/views/right_pane/BUILD | 4 +- .../views/right_pane/right_pane_module.ts | 4 +- .../views/right_pane/right_pane_test.ts | 36 ++++++++------ .../settings_view_component.ng.html | 47 ++++++++++++------- .../right_pane/settings_view_component.scss | 6 +-- .../right_pane/settings_view_component.ts | 2 +- .../webapp/theme/_tb_theme.template.scss | 7 +++ 8 files changed, 78 insertions(+), 39 deletions(-) diff --git a/tensorboard/webapp/angular/BUILD b/tensorboard/webapp/angular/BUILD index 5c710b161a..d5e412ff44 100644 --- a/tensorboard/webapp/angular/BUILD +++ b/tensorboard/webapp/angular/BUILD @@ -249,7 +249,7 @@ tf_ts_library( ], ) -# This is a dummy rule used as a @angular/material/slider dependency. +# This is a dummy rule used as a @angular/material/legacy-slider dependency. tf_ts_library( name = "expect_angular_legacy_material_slider", srcs = [], @@ -258,6 +258,15 @@ tf_ts_library( ], ) +# This is a dummy rule used as a @angular/material/slider dependency. +tf_ts_library( + name = "expect_angular_material_slider", + srcs = [], + deps = [ + "@npm//@angular/material", + ], +) + # This is a dummy rule used as a @angular/material/snackbar dependency. tf_ts_library( name = "expect_angular_material_snackbar", diff --git a/tensorboard/webapp/metrics/views/right_pane/BUILD b/tensorboard/webapp/metrics/views/right_pane/BUILD index 2153af2602..81b68a3598 100644 --- a/tensorboard/webapp/metrics/views/right_pane/BUILD +++ b/tensorboard/webapp/metrics/views/right_pane/BUILD @@ -27,12 +27,12 @@ tf_ng_module( deps = [ "//tensorboard/webapp:app_state", "//tensorboard/webapp:selectors", - "//tensorboard/webapp/angular:expect_angular_legacy_material_slider", "//tensorboard/webapp/angular:expect_angular_material_button", "//tensorboard/webapp/angular:expect_angular_material_button_toggle", "//tensorboard/webapp/angular:expect_angular_material_checkbox", "//tensorboard/webapp/angular:expect_angular_material_icon", "//tensorboard/webapp/angular:expect_angular_material_select", + "//tensorboard/webapp/angular:expect_angular_material_slider", "//tensorboard/webapp/feature_flag", "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/actions", @@ -58,10 +58,10 @@ tf_ts_library( "//tensorboard/webapp:app_state", "//tensorboard/webapp:selectors", "//tensorboard/webapp/angular:expect_angular_core_testing", - "//tensorboard/webapp/angular:expect_angular_legacy_material_slider", "//tensorboard/webapp/angular:expect_angular_material_button_toggle", "//tensorboard/webapp/angular:expect_angular_material_checkbox", "//tensorboard/webapp/angular:expect_angular_material_select", + "//tensorboard/webapp/angular:expect_angular_material_slider", "//tensorboard/webapp/angular:expect_angular_platform_browser_animations", "//tensorboard/webapp/angular:expect_ngrx_store_testing", "//tensorboard/webapp/metrics:types", diff --git a/tensorboard/webapp/metrics/views/right_pane/right_pane_module.ts b/tensorboard/webapp/metrics/views/right_pane/right_pane_module.ts index b7533a7946..06e8efa53f 100644 --- a/tensorboard/webapp/metrics/views/right_pane/right_pane_module.ts +++ b/tensorboard/webapp/metrics/views/right_pane/right_pane_module.ts @@ -19,7 +19,7 @@ import {MatButtonToggleModule} from '@angular/material/button-toggle'; import {MatCheckboxModule} from '@angular/material/checkbox'; import {MatIconModule} from '@angular/material/icon'; import {MatSelectModule} from '@angular/material/select'; -import {MatLegacySliderModule} from '@angular/material/legacy-slider'; +import {MatSliderModule} from '@angular/material/slider'; import {FeatureFlagModule} from '../../../feature_flag/feature_flag_module'; import {DropdownModule} from '../../../widgets/dropdown/dropdown_module'; import {RangeInputModule} from '../../../widgets/range_input/range_input_module'; @@ -42,7 +42,7 @@ import {SettingsViewContainer} from './settings_view_container'; MatCheckboxModule, MatIconModule, MatSelectModule, - MatLegacySliderModule, + MatSliderModule, FeatureFlagModule, RangeInputModule, ], 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 5704b76e86..5501270348 100644 --- a/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts +++ b/tensorboard/webapp/metrics/views/right_pane/right_pane_test.ts @@ -22,7 +22,7 @@ import { import {MatButtonToggleModule} from '@angular/material/button-toggle'; import {MatCheckboxModule} from '@angular/material/checkbox'; import {MatSelectModule} from '@angular/material/select'; -import {MatLegacySliderModule} from '@angular/material/legacy-slider'; +import {MatSliderModule} from '@angular/material/slider'; import {By} from '@angular/platform-browser'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import {Store} from '@ngrx/store'; @@ -50,7 +50,7 @@ describe('metrics right_pane', () => { MatButtonToggleModule, MatCheckboxModule, MatSelectModule, - MatLegacySliderModule, + MatSliderModule, ], declarations: [ RightPaneComponent, @@ -113,8 +113,9 @@ describe('metrics right_pane', () => { }); function getMatSliderValue(el: DebugElement): string { - return el.query(By.css('.mat-slider-thumb-label-text')).nativeElement - .textContent; + return el + .query(By.css('input')) + .nativeElement.getAttribute('aria-valuetext'); } function select( @@ -124,7 +125,7 @@ describe('metrics right_pane', () => { return fixture.debugElement.query(By.css(cssSelector)); } - it('renders', () => { + it('renders', fakeAsync(() => { store.overrideSelector( selectors.getMetricsTooltipSort, TooltipSort.ALPHABETICAL @@ -142,6 +143,8 @@ describe('metrics right_pane', () => { const fixture = TestBed.createComponent(SettingsViewContainer); fixture.detectChanges(); + tick(10); + fixture.detectChanges(); const tooltipSortSelect = select(fixture, '.tooltip-sort tb-dropdown'); // In the test setting, material component's DOM does not reflect the @@ -177,7 +180,7 @@ describe('metrics right_pane', () => { expect(scalarSmoothingInput.nativeElement.value).toBe('0.3'); expect( getMatSliderValue(select(fixture, '.scalars-smoothing mat-slider')) - ).toBe('0.30'); + ).toBe('0.3'); expect( getMatSliderValue(select(fixture, '.image-brightness mat-slider')) @@ -191,7 +194,7 @@ describe('metrics right_pane', () => { select(fixture, '.image-show-actual-size input').componentInstance .checked ).toBeTrue(); - }); + })); it('hides settings if images are not supported', () => { store.overrideSelector(selectors.getIsMetricsImageSupportEnabled, false); @@ -329,9 +332,9 @@ describe('metrics right_pane', () => { it('dispatches metricsChangeCardWidth action when adjusting the slider', fakeAsync(() => { const fixture = TestBed.createComponent(SettingsViewContainer); fixture.detectChanges(); - const slider = select(fixture, CARD_WIDTH_SLIDER); + const sliderThumb = select(fixture, '.card-width mat-slider input'); - slider.triggerEventHandler('input', {value: 350}); + sliderThumb.triggerEventHandler('valueChange', 350); tick(TEST_ONLY.SLIDER_AUDIT_TIME_MS); expect(dispatchSpy).toHaveBeenCalledOnceWith( @@ -351,15 +354,20 @@ describe('metrics right_pane', () => { ); }); - it('sets the card width to the value provided', () => { + it('sets the card width to the value provided', fakeAsync(() => { store.overrideSelector(selectors.getMetricsCardMinWidth, 400); const fixture = TestBed.createComponent(SettingsViewContainer); fixture.detectChanges(); - expect(getMatSliderValue(select(fixture, CARD_WIDTH_SLIDER))).toBe( - '400' - ); - }); + // For some unknown reason sliders which do not display a thumb do not + // update aria-valuetext properly in tests. As a workaround I am using + // the ng-reflect-value attribute for this test. + expect( + select(fixture, CARD_WIDTH_SLIDER) + .query(By.css('input')) + .nativeElement.getAttribute('ng-reflect-value') + ).toBe('400'); + })); it('does not set invalid value', () => { store.overrideSelector(selectors.getMetricsCardMinWidth, null); 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 8df81f50c6..1a9119f073 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 @@ -74,10 +74,13 @@

General

[max]="MAX_CARD_WIDTH_SLIDER_VALUE" [min]="MIN_CARD_WIDTH_SLIDER_VALUE" [step]="50" - [value]="cardMinWidth" - [thumbLabel]="false" - (input)="cardWidthSliderChanged$.emit($event.value)" - > + > + +