fix(material-experimental/mdc-slider): code review changes

* remove circular dependency between MatSlider and MatSliderThumb
* change the selectors for MatSliderThumb
* add a comment describing why and how the value property on the MatSliderThumb works
* delete min, max, and step @inputs from MatSliderThumb
* remove _initialize property from MatSliderThumb
* move all of the MatSliderThumb initialization logic into ngAfterViewInit
* remove input initialization logic from MatSlider ngAfterViewInit
* add input validation logic to MatSlider ngAfterViewInit
* fix value indicator text bug where the DOM was not getting updated

Author: wagnermaciel

src/material-experimental/mdc-slider/slider-adapter.ts

@@ -6,23 +6,51 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ChangeDetectorRef, ElementRef, Inject} from '@angular/core';
 import {SpecificEventListener, EventType} from '@material/base';
 import {MDCSliderAdapter, Thumb, TickMark} from '@material/slider';
-import {MatSlider} from './slider';
+import {MatSliderThumb, MAT_SLIDER} from './slider-thumb';
 
+/**
+ * This is a dummy interface that just contains the properties and methods of MatSlider that are
+ * used by SliderAdapter. Rather than directly referencing MatSlider, we use this interface when
+ * to avoid a circular dependency between MatSlider and SliderAdapter.
+ */
+interface MatSlider {
+  _cdr: ChangeDetectorRef;
+  min: number;
+  max: number;
+  disabled: boolean;
+  _elementRef: ElementRef<HTMLElement>;
+  _trackActive: ElementRef<HTMLElement>;
+  _initialized: boolean;
+  _tickMarks: TickMark[];
+  _document: Document;
+  _window: Window;
+  displayWith: ((value: number) => string) | null;
+  _getInput: (thumb: Thumb) => MatSliderThumb;
+  _getKnobElement: (thumb: Thumb) => HTMLElement;
+  _getThumbElement: (thumb: Thumb) => HTMLElement;
+  _getInputElement: (thumb: Thumb) => HTMLInputElement;
+  _setValue: (value: number, thumb: Thumb) => void;
+  _setValueIndicatorText: (value: number, thumb: Thumb) => void;
+}
+
+// TODO(wagnermaciel): Change to prototype methods once this PR is submitted.
+// https://github.com/material-components/material-components-web/pull/6256
 export class SliderAdapter implements MDCSliderAdapter {
-  constructor(private readonly _delegate: MatSlider) {}
+  constructor(@Inject(MAT_SLIDER) private readonly _delegate: MatSlider) {}
   hasClass = (className: string): boolean => {
-    return this._delegate._hostElement.classList.contains(className);
+    return this._delegate._elementRef.nativeElement.classList.contains(className);
   }
   addClass = (className: string): void => {
-    this._delegate._hostElement.classList.add(className);
+    this._delegate._elementRef.nativeElement.classList.add(className);
   }
   removeClass = (className: string): void => {
-    this._delegate._hostElement.classList.remove(className);
+    this._delegate._elementRef.nativeElement.classList.remove(className);
   }
   getAttribute = (attribute: string): string | null => {
-    return this._delegate._hostElement.getAttribute(attribute);
+    return this._delegate._elementRef.nativeElement.getAttribute(attribute);
   }
   addThumbClass = (className: string, thumb: Thumb): void => {
     this._delegate._getThumbElement(thumb).classList.add(className);
@@ -58,7 +86,7 @@ export class SliderAdapter implements MDCSliderAdapter {
     return this._delegate._getThumbElement(thumb).getBoundingClientRect();
   }
   getBoundingClientRect = (): ClientRect => {
-    return this._delegate._hostElement.getBoundingClientRect();
+    return this._delegate._elementRef.nativeElement.getBoundingClientRect();
   }
   isRTL = (): boolean => {
     // TODO(wagnermaciel): Actually implementing this.
@@ -84,10 +112,10 @@ export class SliderAdapter implements MDCSliderAdapter {
   }
   updateTickMarks = (tickMarks: TickMark[]): void => {
     this._delegate._tickMarks = tickMarks;
-    this._delegate._cdr.markForCheck();
+    this._delegate._cdr.detectChanges();
   }
   setPointerCapture = (pointerId: number): void => {
-    this._delegate._hostElement.setPointerCapture(pointerId);
+    this._delegate._elementRef.nativeElement.setPointerCapture(pointerId);
   }
   // We ignore emitChangeEvent and emitInputEvent because the slider inputs
   // are already exposed so users can just listen for those events directly themselves.
@@ -113,11 +141,11 @@ export class SliderAdapter implements MDCSliderAdapter {
   }
   registerEventHandler =
     <K extends EventType>(evtType: K, handler: SpecificEventListener<K>): void => {
-      this._delegate._hostElement.addEventListener(evtType, handler);
+      this._delegate._elementRef.nativeElement.addEventListener(evtType, handler);
   }
   deregisterEventHandler =
     <K extends EventType>(evtType: K, handler: SpecificEventListener<K>): void => {
-      this._delegate._hostElement.removeEventListener(evtType, handler);
+      this._delegate._elementRef.nativeElement.removeEventListener(evtType, handler);
   }
   registerThumbEventHandler =
     <K extends EventType>(thumb: Thumb, evtType: K, handler: SpecificEventListener<K>): void => {

src/material-experimental/mdc-slider/slider-thumb.ts

@@ -6,11 +6,38 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {NumberInput} from '@angular/cdk/coercion';
+import {coerceNumberProperty, NumberInput} from '@angular/cdk/coercion';
 import {DOCUMENT} from '@angular/common';
-import {Directive, ElementRef, EventEmitter, Inject, Input, Output} from '@angular/core';
+import {
+  AfterViewInit,
+  Directive,
+  ElementRef,
+  EventEmitter,
+  Inject,
+  InjectionToken,
+  Input,
+  Output,
+} from '@angular/core';
 import {Thumb} from '@material/slider';
-import {MatSlider} from './slider';
+
+/**
+ * This is a dummy interface that just contains the properties and methods of MatSlider that are
+ * used by MatSliderThumb. Rather than directly referencing MatSlider, we use this interface when
+ * defining MAT_SLIDER to avoid a circular dependency between MatSlider and MatSliderThumb.
+ */
+interface MatSlider {
+  min: number;
+  max: number;
+  disabled: boolean;
+  _initialized: boolean;
+  _getInput: (thumb: Thumb) => MatSliderThumb;
+  _setValue: (value: number, thumb: Thumb) => void;
+}
+
+/**
+ * Injection token that can be used to inject instances of MatSlider.
+ */
+export const MAT_SLIDER = new InjectionToken<MatSlider>('MatSlider');
 
 /**
  * Represents a drag event emitted by the MatSlider component.
@@ -33,61 +60,44 @@ export interface MatSliderDragEvent {
  * The native input used by the MatSlider.
  */
 @Directive({
-  selector: 'input[mat-slider-thumb]',
+  selector: 'input[matSliderThumb], input[matSliderStartThumb], input[matSliderEndThumb]',
   host: {
     'class': 'mdc-slider__input',
     'type': 'range',
-    '[min]': 'min',
-    '[max]': 'max',
-    '[step]': 'step',
-    '[attr.value]': 'value',
     '(blur)': '_blur.emit()',
     '(focus)': '_focus.emit()',
-  }
-}) export class MatSliderThumb {
+  },
+}) export class MatSliderThumb implements AfterViewInit {
+
+  // ** IMPORTANT NOTE **
+  //
+  // The way `value` is implemented for MatSliderThumb goes against our standard practice. Normally
+  // we would define a private variable `_value` as the source of truth for the value of the slider
+  // thumb input. The source of truth for the value of the slider inputs has already been decided
+  // for us by MDC to be the value attribute on the slider thumb inputs. This is because the MDC
+  // foundation and adapter expect that the value attribute is the source of truth for the slider
+  // inputs.
+  //
+  // Also, note that the value attribute is completely disconnected from the value property.
+
   /** The current value of this slider input. */
   @Input()
-  get value(): number { return Number(this._elementRef.nativeElement.getAttribute('value')); }
+  get value(): number {
+    return coerceNumberProperty(this._elementRef.nativeElement.getAttribute('value'));
+  }
   set value(v: number) {
-    this._initialized = true;
+    const value = coerceNumberProperty(v);
 
     // If the foundation has already been initialized, we need to
     // relay any value updates to it so that it can update the UI.
     if (this._slider._initialized) {
-      this._slider._setValue(v, this.thumb);
+      this._slider._setValue(value, this.thumb);
     } else {
       // Setup for the MDC foundation.
-      this._elementRef.nativeElement.setAttribute('value', v.toString());
+      this._elementRef.nativeElement.setAttribute('value', `${value}`);
     }
   }
 
-  /** The minimum value that this slider input can have. */
-  @Input()
-  get min(): number {
-    return (this._slider._isRange() && this.thumb === Thumb.END)
-      ? this._slider._getValue(Thumb.START)
-      : this._slider.min;
-  }
-  set min(v: number) { throw Error('Invalid attribute "min" on MatSliderThumb.'); }
-
-  /** The maximum value that this slider input can have. */
-  @Input()
-  get max(): number {
-    return (this._slider._isRange() && this.thumb === Thumb.START)
-      ? this._slider._getValue(Thumb.END)
-      : this._slider.max;
-  }
-  set max(v: number) { throw Error('Invalid attribute "max" on MatSliderThumb.'); }
-
-  /** The size of each increment between the values of the slider. */
-  @Input()
-  get step(): number { return 1; }
-  set step(v: number) { throw Error('Invalid attribute "step" on MatSliderThumb.'); }
-
-  /** MDC Slider does not use the disabled attribute it's native inputs. */
-  @Input()
-  set disabled(v: boolean) { throw Error('Invalid attribute "disabled" on MatSliderThumb.'); }
-
   /** Event emitted when the slider thumb starts being dragged. */
   @Output() readonly dragStart: EventEmitter<MatSliderDragEvent>
     = new EventEmitter<MatSliderDragEvent>();
@@ -105,35 +115,29 @@ export interface MatSliderDragEvent {
   /** Indicates which slider thumb this input corresponds to. */
   thumb: Thumb;
 
-  /** Whether the value of this slider thumb input has been set. */
-  _initialized: boolean = false;
-
   constructor(
     @Inject(DOCUMENT) private readonly _document: Document,
+    @Inject(MAT_SLIDER) private readonly _slider: MatSlider,
     readonly _elementRef: ElementRef<HTMLInputElement>,
-    private readonly _slider: MatSlider,
-    ) {
-      // Initializing the min and max in the constructor guarantees that they will be
-      // defined by the time the value gets set. If the range is not defined before we
-      // try to set the value, we can run into the issue where the value is outside of
-      // the default range and get capped to the default min or max.
-      this._elementRef.nativeElement.min = this._slider.min.toString();
-      this._elementRef.nativeElement.max = this._slider.max.toString();
-    }
+    ) {}
+
+  ngAfterViewInit() {
+    this.thumb = this._elementRef.nativeElement.hasAttribute('matSliderStartThumb')
+    ? Thumb.START
+    : Thumb.END;
+
+    const min = this._elementRef.nativeElement.hasAttribute('matSliderEndThumb')
+      ? this._slider._getInput(Thumb.START).value
+      : this._slider.min;
+    const max = this._elementRef.nativeElement.hasAttribute('matSliderStartThumb')
+      ? this._slider._getInput(Thumb.END).value
+      : this._slider.max;
+    this._elementRef.nativeElement.min = `${min}`;
+    this._elementRef.nativeElement.max = `${max}`;
 
-  /**
-   * Sets up the initial state of the slider thumb input.
-   *
-   * This is needed because the slider thumb input is passed in via `ng-content`,
-   * and therefore has no way of knowing which slider thumb it correspond to.
-   */
-  _init(thumb: Thumb): void {
-    this.thumb = thumb;
-
-    // If the value has not been initialized (i.e. no value was provided from
-    // the user), determine the default value for the slider based on the given thumb.
-    if (!this._initialized) {
-      this.value = (this._slider._isRange() && thumb === Thumb.END)
+    // Only set the default value if an initial value has not already been provided.
+    if (!this._elementRef.nativeElement.hasAttribute('value')) {
+      this.value = this._elementRef.nativeElement.hasAttribute('matSliderEndThumb')
         ? this._slider.max
         : this._slider.min;
     }

src/material-experimental/mdc-slider/slider.html

@@ -16,9 +16,7 @@
 <div class="mdc-slider__thumb" *ngFor="let thumb of _getThumbTypes()" #thumb>
   <div class="mdc-slider__value-indicator-container" *ngIf="discrete">
     <div class="mdc-slider__value-indicator">
-      <span class="mdc-slider__value-indicator-text">
-        {{_getValueIndicatorTextByThumb(thumb)}}
-      </span>
+      <span class="mdc-slider__value-indicator-text" #valueIndicatorTextElement></span>
     </div>
   </div>
   <div class="mdc-slider__thumb-knob" #knob></div>