Skip to content

Commit 62497a0

Browse files
committed
fix(material-experimental/mdc-slider): code review changes
* make input validation only happen in ngDevMode * move _validateInputs() and _throwInvalidInputConfigurationError() out of MatSlider so they can be tree-shaken away in production mode * delete _getValueIndicatorText() and _getValueIndicatorTextElement() and add their logic to _setValueIndicatorText() * made getters more efficient * deleted unused _getValue() method * move export class MatSliderThumb ... to its own separate line * moved MatSliderThumb value initialization to the constructor to avoid race conditions when setting the min and max * set the value property in ngAfterViewInit for MatSliderThumb
1 parent 238ffea commit 62497a0

File tree

2 files changed

+77
-76
lines changed

2 files changed

+77
-76
lines changed

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

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ export interface MatSliderDragEvent {
6767
'(blur)': '_blur.emit()',
6868
'(focus)': '_focus.emit()',
6969
},
70-
}) export class MatSliderThumb implements AfterViewInit {
70+
})
71+
export class MatSliderThumb implements AfterViewInit {
7172

7273
// ** IMPORTANT NOTE **
7374
//
@@ -119,13 +120,22 @@ export interface MatSliderDragEvent {
119120
@Inject(DOCUMENT) private readonly _document: Document,
120121
@Inject(MAT_SLIDER) private readonly _slider: MatSlider,
121122
readonly _elementRef: ElementRef<HTMLInputElement>,
122-
) {}
123+
) {
124+
this.thumb = _elementRef.nativeElement.hasAttribute('matSliderStartThumb')
125+
? Thumb.START
126+
: Thumb.END;
127+
128+
// Only set the default value if an initial value has not already been provided.
129+
// Note that we are only setting the value attribute at this point. We cannot set the value
130+
// property yet because the min and max have not been set.
131+
if (!_elementRef.nativeElement.hasAttribute('value')) {
132+
this.value = _elementRef.nativeElement.hasAttribute('matSliderEndThumb')
133+
? _slider.max
134+
: _slider.min;
135+
}
136+
}
123137

124138
ngAfterViewInit() {
125-
this.thumb = this._elementRef.nativeElement.hasAttribute('matSliderStartThumb')
126-
? Thumb.START
127-
: Thumb.END;
128-
129139
const min = this._elementRef.nativeElement.hasAttribute('matSliderEndThumb')
130140
? this._slider._getInput(Thumb.START).value
131141
: this._slider.min;
@@ -135,12 +145,8 @@ export interface MatSliderDragEvent {
135145
this._elementRef.nativeElement.min = `${min}`;
136146
this._elementRef.nativeElement.max = `${max}`;
137147

138-
// Only set the default value if an initial value has not already been provided.
139-
if (!this._elementRef.nativeElement.hasAttribute('value')) {
140-
this.value = this._elementRef.nativeElement.hasAttribute('matSliderEndThumb')
141-
? this._slider.max
142-
: this._slider.min;
143-
}
148+
// We can now set the property value because the min and max have now been set.
149+
this._elementRef.nativeElement.value = `${this.value}`;
144150

145151
// Setup for the MDC foundation.
146152
if (this._slider.disabled) {

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

Lines changed: 59 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,13 @@ export class MatSlider implements AfterViewInit, OnDestroy {
150150
}
151151

152152
ngAfterViewInit() {
153-
this._validateInputs();
153+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
154+
_validateInputs(
155+
this._isRange(),
156+
this._getInputElement(Thumb.START),
157+
this._getInputElement(Thumb.END),
158+
);
159+
}
154160
if (this._platform.isBrowser) {
155161
this._foundation.init();
156162
this._foundation.layout();
@@ -164,31 +170,6 @@ export class MatSlider implements AfterViewInit, OnDestroy {
164170
}
165171
}
166172

167-
/**
168-
* Ensures that there is not an invalid configuration for the slider thumb inputs.
169-
*/
170-
_validateInputs(): void {
171-
if (this._isRange()) {
172-
if (!this._getInputElement(Thumb.START).hasAttribute('matSliderStartThumb')) {
173-
this._throwInvalidInputConfigurationError('matSliderStartThumb');
174-
}
175-
if (!this._getInputElement(Thumb.END).hasAttribute('matSliderEndThumb')) {
176-
this._throwInvalidInputConfigurationError('matSliderEndThumb');
177-
}
178-
} else {
179-
if (!this._getInputElement(Thumb.END).hasAttribute('matSliderThumb')) {
180-
this._throwInvalidInputConfigurationError('matSliderThumb');
181-
}
182-
}
183-
}
184-
185-
/** Gets the current value of given slider thumb. */
186-
_getValue(thumb: Thumb): number {
187-
return thumb === Thumb.START
188-
? this._foundation.getValueStart()
189-
: this._foundation.getValue();
190-
}
191-
192173
/** Sets the value of a slider thumb. */
193174
_setValue(value: number, thumb: Thumb): void {
194175
thumb === Thumb.START
@@ -203,7 +184,7 @@ export class MatSlider implements AfterViewInit, OnDestroy {
203184

204185
/** Gets the slider thumb input of the given thumb. */
205186
_getInput(thumb: Thumb): MatSliderThumb {
206-
return thumb === Thumb.END ? this._inputs.get(this._inputs.length - 1)! : this._inputs.get(0)!;
187+
return thumb === Thumb.END ? this._inputs.last! : this._inputs.first!;
207188
}
208189

209190
/** Gets the slider thumb HTML input element of the given thumb. */
@@ -213,35 +194,28 @@ export class MatSlider implements AfterViewInit, OnDestroy {
213194

214195
/** Gets the slider thumb HTML element of the given thumb. */
215196
_getThumbElement(thumb: Thumb): HTMLElement {
216-
const thumbs = this._thumbs.toArray().map(e => e.nativeElement);
217-
return thumb === Thumb.END ? thumbs[thumbs.length - 1] : thumbs[0];
197+
const thumbElementRef = thumb === Thumb.END ? this._thumbs.last : this._thumbs.first;
198+
return thumbElementRef.nativeElement;
218199
}
219200

220201
/** Gets the slider knob HTML element of the given thumb. */
221202
_getKnobElement(thumb: Thumb): HTMLElement {
222-
const knobs = this._knobs.toArray().map(e => e.nativeElement);
223-
return thumb === Thumb.END ? knobs[knobs.length - 1] : knobs[0];
203+
const knobElementRef = thumb === Thumb.END ? this._knobs.last : this._knobs.first;
204+
return knobElementRef.nativeElement;
224205
}
225206

226-
/** Gets the slider knob HTML element of the given thumb. */
227-
_getValueIndicatorTextElement(thumb: Thumb): HTMLElement {
228-
const elements = this._valueIndicatorTextElements.toArray().map(e => e.nativeElement);
229-
return thumb === Thumb.END ? elements[elements.length - 1] : elements[0];
230-
}
231-
232207
/**
233-
* Gets the text representation of the given value.
208+
* Sets the value indicator text of the given thumb using the given value.
234209
*
235-
* Uses the `displayWith` function if one has been provided. Otherwise, it just returns the
236-
* current numeric value as a string.
210+
* Uses the `displayWith` function if one has been provided. Otherwise, it just uses the
211+
* numeric value as a string.
237212
*/
238-
_getValueIndicatorText(value: number): string {
239-
return this.displayWith ? this.displayWith(value) : `${value}`;
240-
}
241-
242-
/** Sets the value indicator text of the given thumb with the given value. */
243213
_setValueIndicatorText(value: number, thumb: Thumb): void {
244-
this._getValueIndicatorTextElement(thumb).textContent = this._getValueIndicatorText(value);
214+
const valueIndicatorTextElementRef = thumb === Thumb.END
215+
? this._valueIndicatorTextElements.last
216+
: this._valueIndicatorTextElements.first;
217+
const valueText = this.displayWith ? this.displayWith(value) : `${value}`;
218+
valueIndicatorTextElementRef.nativeElement.textContent = valueText;
245219
}
246220

247221
/** Determines the class name for a HTML element. */
@@ -256,28 +230,49 @@ export class MatSlider implements AfterViewInit, OnDestroy {
256230
return this._isRange() ? [Thumb.START, Thumb.END] : [Thumb.END];
257231
}
258232

259-
_throwInvalidInputConfigurationError(missingSelector: string): void {
260-
throw Error(`Invalid slider thumb input configuration! Missing a ${missingSelector}.
261-
262-
Valid configurations are as follows:
263-
264-
<mat-slider>
265-
<input mat-slider-thumb>
266-
</mat-slider>
267-
268-
or
269-
270-
<mat-slider>
271-
<input matSliderStartThumb>
272-
<input matSliderEndThumb>
273-
</mat-slider>
274-
`);
275-
}
276-
277233
static ngAcceptInputType_disabled: BooleanInput;
278234
static ngAcceptInputType_discrete: BooleanInput;
279235
static ngAcceptInputType_showTickMarks: BooleanInput;
280236
static ngAcceptInputType_min: NumberInput;
281237
static ngAcceptInputType_max: NumberInput;
282238
static ngAcceptInputType_step: NumberInput;
283239
}
240+
241+
/**
242+
* Ensures that there is not an invalid configuration for the slider thumb inputs.
243+
*/
244+
function _validateInputs(
245+
isRange: boolean,
246+
startInputElement: HTMLInputElement,
247+
endInputElement: HTMLInputElement): void {
248+
if (isRange) {
249+
if (!startInputElement!.hasAttribute('matSliderStartThumb')) {
250+
_throwInvalidInputConfigurationError();
251+
}
252+
if (!endInputElement.hasAttribute('matSliderEndThumb')) {
253+
_throwInvalidInputConfigurationError();
254+
}
255+
} else {
256+
if (!endInputElement.hasAttribute('matSliderThumb')) {
257+
_throwInvalidInputConfigurationError();
258+
}
259+
}
260+
}
261+
262+
function _throwInvalidInputConfigurationError(): void {
263+
throw Error(`Invalid slider thumb input configuration!
264+
265+
Valid configurations are as follows:
266+
267+
<mat-slider>
268+
<input matSliderThumb>
269+
</mat-slider>
270+
271+
or
272+
273+
<mat-slider>
274+
<input matSliderStartThumb>
275+
<input matSliderEndThumb>
276+
</mat-slider>
277+
`);
278+
}

0 commit comments

Comments
 (0)