Skip to content

Commit

Permalink
MDC Migration: migrate mat-slider in image card (#6666)
Browse files Browse the repository at this point in the history
## Motivation for features / changes
Angular mandated migration to new MDC components.

## Technical description of changes
Migrating the component was straightforward. The difficult piece was to
adjust to match the custom tick highlights which indicate if the image
is within the range or selection from linked time. We used to highlight
the entire slider within the range. I do not find that to be useful and
so I removed it.

## Screenshots of UI changes (or N/A)
Without linked time:
<img width="820" alt="Screenshot 2023-10-25 at 3 40 19 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/8cc28c6c-471e-4f00-a557-589fe0d022e0">
<img width="820" alt="Screenshot 2023-10-25 at 3 40 29 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/4fc22308-8787-4cba-b15a-a42b282a50ba">

With linked time and a range from 0-4:
<img width="821" alt="Screenshot 2023-10-25 at 3 33 51 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/c5f63b41-e93f-4332-89d4-69a7a589a8b5">
<img width="825" alt="Screenshot 2023-10-25 at 3 34 38 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/45d8ecf2-f0e2-4259-95cb-0f5f1682fe82">
  • Loading branch information
JamesHollyer authored Oct 27, 2023
1 parent 9b21992 commit 49999d6
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 243 deletions.
4 changes: 2 additions & 2 deletions tensorboard/webapp/metrics/views/card_renderer/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ tf_ng_module(
":utils",
":vis_linked_time_selection_warning",
"//tensorboard/webapp:app_state",
"//tensorboard/webapp/angular:expect_angular_legacy_material_slider",
"//tensorboard/webapp/angular:expect_angular_material_button",
"//tensorboard/webapp/angular:expect_angular_material_icon",
"//tensorboard/webapp/angular:expect_angular_material_progress_spinner",
"//tensorboard/webapp/angular:expect_angular_material_slider",
"//tensorboard/webapp/feature_flag/directives",
"//tensorboard/webapp/metrics:types",
"//tensorboard/webapp/metrics/actions",
Expand Down Expand Up @@ -449,13 +449,13 @@ tf_ts_library(
"//tensorboard/webapp:selectors",
"//tensorboard/webapp/angular:expect_angular_cdk_overlay",
"//tensorboard/webapp/angular:expect_angular_core_testing",
"//tensorboard/webapp/angular:expect_angular_legacy_material_slider",
"//tensorboard/webapp/angular:expect_angular_material_button",
"//tensorboard/webapp/angular:expect_angular_material_dialog",
"//tensorboard/webapp/angular:expect_angular_material_input",
"//tensorboard/webapp/angular:expect_angular_material_menu",
"//tensorboard/webapp/angular:expect_angular_material_progress_spinner",
"//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/experiments:types",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,24 +81,18 @@
<mat-slider
class="step-slider"
color="primary"
[ngClass]="[linkedTimeSelection && linkedTimeSelection.endStep !== null ? 'hide-slider' : '']"
[disabled]="steps.length <= 1"
[min]="0"
[max]="steps.length - 1"
[step]="1"
[tickInterval]="1"
[value]="stepIndex"
(input)="onSliderInput($event)"
></mat-slider>
>
<input
matSliderThumb
(valueChange)="onSliderInput($event)"
[value]="stepIndex"
/>
</mat-slider>
<div class="linked-time-wrapper" *ngIf="linkedTimeSelection">
<span *ngIf="linkedTimeSelection.endStep !== null">
<span class="slider-track"></span>
<span
class="slider-track-fill"
[style.left]="sliderStartPosition"
[style.width]="sliderTrackWidth"
></span>
</span>
<div
class="linked-time-tick"
*ngFor="let step of selectedSteps"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
@import '../common';

$_title-to-heading-gap: 12px;
$_tick_width: 14px;

:host {
box-sizing: border-box;
Expand Down Expand Up @@ -143,6 +142,11 @@ $_tick_width: 14px;
// https://github.com/angular/components/blob/master/src/material/slider/slider.scss#L10
height: 24px;
position: relative;

mat-slider {
margin-left: 6px;
margin-right: 6px;
}
}

.step-slider {
Expand Down Expand Up @@ -171,28 +175,8 @@ $_tick_width: 14px;
.linked-time-tick {
@include tb-theme-background-prop(background-color, selected-button);
border-radius: 50%;
height: 14px;
position: absolute;
width: 14px;
}

.slider-track,
.slider-track-fill {
height: 2px;
top: 6px;
height: 12px;
position: absolute;
}

.slider-track {
@include tb-theme-foreground-prop(background, slider-off);
left: $_tick_width / 2;
width: calc(100% - #{$_tick_width});
}

.slider-track-fill {
background: mat.get-color-from-palette($tb-primary);

@include tb-dark-theme {
background: mat.get-color-from-palette($tb-dark-primary);
}
width: 12px;
z-index: -1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {DataLoadState} from '../../../types/data';
import {RunColorScale} from '../../../types/ui';
import {TimeSelectionView} from './utils';

const TICK_WIDTH = 14; // In px
const TICK_WIDTH = 12; // In px

@Component({
selector: 'image-card-component',
Expand Down Expand Up @@ -70,12 +70,8 @@ export class ImageCardComponent {
return `contrast(${contrastPercent}%) brightness(${brightnessScale})`;
}

onSliderInput($event: any) {
// Angular Material Slider's MatLegacySliderChange has a loose `number | null`
// type for 'value'. However, it's actual implementation can only emit a
// `number` on input events.
// https://github.com/angular/components/blob/master/src/material/slider/slider.ts
this.stepIndexChange.emit($event.value as number);
onSliderInput(value: number) {
this.stepIndexChange.emit(value);
}

changeDistinct(change: SimpleChange) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {NgModule} from '@angular/core';
import {MatButtonModule} from '@angular/material/button';
import {MatIconModule} from '@angular/material/icon';
import {MatProgressSpinnerModule} from '@angular/material/progress-spinner';
import {MatLegacySliderModule} from '@angular/material/legacy-slider';
import {MatSliderModule} from '@angular/material/slider';
import {TruncatedPathModule} from '../../../widgets/text/truncated_path_module';
import {ImageCardComponent} from './image_card_component';
import {ImageCardContainer} from './image_card_container';
Expand All @@ -34,7 +34,7 @@ import {VisLinkedTimeSelectionWarningModule} from './vis_linked_time_selection_w
MatButtonModule,
MatIconModule,
MatProgressSpinnerModule,
MatLegacySliderModule,
MatSliderModule,
RunNameModule,
TruncatedPathModule,
VisLinkedTimeSelectionWarningModule,
Expand Down
Loading

0 comments on commit 49999d6

Please sign in to comment.