MDCMigration: Migrate mat slider in settingsViewComponent

tensorflow · Oct 19, 2023 · 38ebe63 · 38ebe63
1 parent 717dec8
commit 38ebe63
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -74,10 +74,13 @@ <h3 class="section-title">General</h3>
         [max]="MAX_CARD_WIDTH_SLIDER_VALUE"
         [min]="MIN_CARD_WIDTH_SLIDER_VALUE"
         [step]="50"
-        [value]="cardMinWidth"
-        [thumbLabel]="false"
-        (input)="cardWidthSliderChanged$.emit($event.value)"
-      ></mat-slider>
+      >
+        <input
+          matSliderThumb
+          (valueChange)="cardWidthSliderChanged$.emit($event)"
+          [value]="cardMinWidth"
+        />
+      </mat-slider>
       <button
         class="reset-button"
         mat-icon-button
@@ -104,10 +107,14 @@ <h3 class="section-title">Scalars</h3>
         [max]="MAX_SMOOTHING_SLIDER_VALUE"
         [min]="0"
         [step]="0.01"
-        [value]="scalarSmoothing"
-        [thumbLabel]="true"
-        (input)="scalarSmoothingControlChanged$.emit($event.value)"
-      ></mat-slider>
+        discrete
+      >
+        <input
+          matSliderThumb
+          (valueChange)="scalarSmoothingControlChanged$.emit($event)"
+          [value]="scalarSmoothing"
+        />
+      </mat-slider>
       <input
         class="slider-input"
         aria-labelledby="scalars-smoothing-label"
@@ -180,11 +187,15 @@ <h3 class="section-title">Images</h3>
         [max]="2000"
         [min]="0"
         [step]="10"
-        [value]="imageBrightnessInMilli"
-        [thumbLabel]="true"
         [displayWith]="formatMilliToZeroth"
-        (input)="imageBrightnessSliderChanged$.emit($event.value)"
-      ></mat-slider>
+        discrete
+      >
+        <input
+          matSliderThumb
+          (valueChange)="imageBrightnessSliderChanged$.emit($event)"
+          [value]="imageBrightnessInMilli"
+        />
+      </mat-slider>
       <button
         class="reset-button"
         mat-icon-button
@@ -207,11 +218,15 @@ <h3 class="section-title">Images</h3>
         [max]="5000"
         [min]="0"
         [step]="10"
-        [value]="imageContrastInMilli"
-        [thumbLabel]="true"
         [displayWith]="formatMilliToZeroth"
-        (input)="imageContrastSliderChanged$.emit($event.value)"
-      ></mat-slider>
+        discrete
+      >
+        <input
+          matSliderThumb
+          (valueChange)="imageContrastSliderChanged$.emit($event)"
+          [value]="imageContrastInMilli"
+        />
+      </mat-slider>
       <button
         class="reset-button"
         mat-icon-button

diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.scss b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.scss
@@ -109,9 +109,9 @@ mat-checkbox {
 mat-slider {
   flex: 1;
   // Reset mat-slider's internal extra space on left/right sides
-  // https://github.com/angular/components/blob/master/src/material/slider/slider.scss#L10
-  margin-left: -8px;
-  margin-right: -8px;
+  // https://github.com/angular/components/blob/8a0196786bb6aa09e17ad84c1a2f035fea4fac5a/src/material/slider/slider.scss#L39
+  margin-left: 0px;
+  margin-right: 0px;
 }
 
 .column-edit-menu-toggle {

diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts
@@ -122,7 +122,7 @@ export class SettingsViewComponent {
   readonly MAX_SMOOTHING_SLIDER_VALUE = MAX_SMOOTHING_SLIDER_VALUE;
 
   readonly scalarSmoothingControlChanged$ = new EventEmitter<number>();
-  @Input() scalarSmoothing!: number;
+  @Input() scalarSmoothing: number = 10;
   @Output()
   scalarSmoothingChanged = this.scalarSmoothingControlChanged$.pipe(
     auditTime(SLIDER_AUDIT_TIME_MS)

diff --git a/tensorboard/webapp/theme/_tb_theme.template.scss b/tensorboard/webapp/theme/_tb_theme.template.scss
@@ -293,6 +293,13 @@ $tb-dark-theme: map_merge(
 
   body,
   body.dark-mode {
+    mat-slider {
+      --mdc-slider-handle-width: 12px;
+      --mdc-slider-handle-height: 12px;
+      --mdc-slider-active-track-height: 2px;
+      --mdc-slider-inactive-track-height: 2px;
+    }
+
     a,
     button.mat-mdc-button-base {
       --tb-icon-size: 24px;