From ad664e633059ed558f037218730067b630f3bda3 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Tue, 23 May 2023 14:45:33 -0400 Subject: [PATCH 1/9] fix(material/slider): handle ngModel initial null value * fixes yaqs/6342072129453817856 --- src/material/slider/slider-input.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index 6b8171f8cc0e..3e257b2dceda 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -243,7 +243,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA _skipUIUpdate: boolean = false; /** Callback called when the slider input value changes. */ - private _onChangeFn: (value: any) => void = () => {}; + protected _onChangeFn: ((value: any) => void) | undefined; /** Callback called when the slider input has been touched. */ private _onTouchedFn: () => void = () => {}; @@ -328,7 +328,9 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA } _onInput(): void { - this._onChangeFn(this.value); + if (this._onChangeFn) { + this._onChangeFn(this.value); + } // handles arrowing and updating the value when // a step is defined. if (this._slider.step || !this._isActive) { @@ -422,7 +424,9 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA this.value = value; this.valueChange.emit(this.value); - this._onChangeFn(this.value); + if (this._onChangeFn) { + this._onChangeFn(this.value); + } this._slider._onValueChange(this); this._slider.step > 0 ? this._updateThumbUIByValue() @@ -500,7 +504,9 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA * @docs-private */ writeValue(value: any): void { - this.value = value; + if (this._onChangeFn) { + this.value = value; + } } /** @@ -738,8 +744,10 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan * @docs-private */ override writeValue(value: any): void { - this.value = value; - this._updateWidthInactive(); - this._updateSibling(); + if (this._onChangeFn) { + this.value = value; + this._updateWidthInactive(); + this._updateSibling(); + } } } From f495366ee7c4ef7a555d9af614ee117cd7ed41f9 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Tue, 23 May 2023 14:52:08 -0400 Subject: [PATCH 2/9] fixup! fix(material/slider): handle ngModel initial null value --- tools/public_api_guard/material/slider.md | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/public_api_guard/material/slider.md b/tools/public_api_guard/material/slider.md index 8037af2b8a8e..68f4bfa071ea 100644 --- a/tools/public_api_guard/material/slider.md +++ b/tools/public_api_guard/material/slider.md @@ -232,6 +232,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA _onBlur(): void; // (undocumented) _onChange(): void; + protected _onChangeFn: ((value: any) => void) | undefined; // (undocumented) _onFocus(): void; // (undocumented) From 6a3ba7aca6297a1a677022f7dbda8aba7c03b112 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Tue, 23 May 2023 19:50:59 -0400 Subject: [PATCH 3/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider-input.ts | 4 +--- src/material/slider/slider.spec.ts | 32 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index 3e257b2dceda..ceeda54ceb2b 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -328,9 +328,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA } _onInput(): void { - if (this._onChangeFn) { - this._onChangeFn(this.value); - } + this._onChangeFn?.(this.value); // handles arrowing and updating the value when // a step is defined. if (this._slider.step || !this._isActive) { diff --git a/src/material/slider/slider.spec.ts b/src/material/slider/slider.spec.ts index a0e4b4389cc6..d3e1ad20c7d2 100644 --- a/src/material/slider/slider.spec.ts +++ b/src/material/slider/slider.spec.ts @@ -1168,6 +1168,22 @@ describe('MDC-based MatSlider', () => { })); }); + describe('range slider w/ NgModel edge case', () => { + it('should initialize correctly despite NgModel `null` bug', fakeAsync(() => { + const fixture = createComponent(RangeSliderWithNgModelEdgeCase); + fixture.detectChanges(); + const sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); + const slider = sliderDebugElement.componentInstance; + const startInput = slider._getInput(_MatThumb.START) as MatSliderRangeThumb; + const endInput = slider._getInput(_MatThumb.END) as MatSliderRangeThumb; + flush(); + console.log('result: ', startInput.value); + checkInput(startInput, {min: -1, max: -0.3, value: -0.7, translateX: 90}); + checkInput(endInput, {min: -0.7, max: 0, value: -0.3, translateX: 210}); + expect(1).toBe(2); + })); + }); + describe('slider as a custom form control', () => { let fixture: ComponentFixture; let slider: MatSlider; @@ -1617,6 +1633,22 @@ class RangeSliderWithNgModel { endVal: number | undefined = 100; } +@Component({ + template: ` + + + + + +`, + styles: SLIDER_STYLES, +}) +class RangeSliderWithNgModelEdgeCase { + @ViewChild(MatSlider) slider: MatSlider; + startValue: number = -0.7; + endValue: number = -0.3; +} + @Component({ template: ` From 4f2acad276940466efebfeaad7874c1f1831149d Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Tue, 23 May 2023 21:33:41 -0400 Subject: [PATCH 4/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/material/slider/slider.spec.ts b/src/material/slider/slider.spec.ts index d3e1ad20c7d2..00fa6f063a67 100644 --- a/src/material/slider/slider.spec.ts +++ b/src/material/slider/slider.spec.ts @@ -1180,7 +1180,6 @@ describe('MDC-based MatSlider', () => { console.log('result: ', startInput.value); checkInput(startInput, {min: -1, max: -0.3, value: -0.7, translateX: 90}); checkInput(endInput, {min: -0.7, max: 0, value: -0.3, translateX: 210}); - expect(1).toBe(2); })); }); From 607506f00fc3d87b4589cc562de91ac0c5936ca9 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 24 May 2023 08:49:37 -0400 Subject: [PATCH 5/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider-input.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index ceeda54ceb2b..019be2d44605 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -248,6 +248,16 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA /** Callback called when the slider input has been touched. */ private _onTouchedFn: () => void = () => {}; + /** + * Whether the NgModel has been initialized. + * + * This flag is used to ignore ghost null calls to + * writeValue which can break slider initialization. + * + * See https://github.com/angular/angular/issues/14988. + */ + protected _isControlInitialized = false; + constructor( readonly _ngZone: NgZone, readonly _elementRef: ElementRef, @@ -422,8 +432,8 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA this.value = value; this.valueChange.emit(this.value); - if (this._onChangeFn) { - this._onChangeFn(this.value); + if (this._isControlInitialized) { + this._onChangeFn!(this.value); } this._slider._onValueChange(this); this._slider.step > 0 @@ -502,7 +512,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA * @docs-private */ writeValue(value: any): void { - if (this._onChangeFn) { + if (this._isControlInitialized) { this.value = value; } } @@ -514,6 +524,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA */ registerOnChange(fn: any): void { this._onChangeFn = fn; + this._isControlInitialized = true; } /** @@ -742,7 +753,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan * @docs-private */ override writeValue(value: any): void { - if (this._onChangeFn) { + if (this._isControlInitialized) { this.value = value; this._updateWidthInactive(); this._updateSibling(); From d63a8fa897fb07693677a5aa26ac47dcb4d7cf82 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 24 May 2023 11:11:22 -0400 Subject: [PATCH 6/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider-input.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index 019be2d44605..0d6544c5425a 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -432,9 +432,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA this.value = value; this.valueChange.emit(this.value); - if (this._isControlInitialized) { - this._onChangeFn!(this.value); - } + this._onChangeFn!(this.value); this._slider._onValueChange(this); this._slider.step > 0 ? this._updateThumbUIByValue() @@ -512,7 +510,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA * @docs-private */ writeValue(value: any): void { - if (this._isControlInitialized) { + if (this._isControlInitialized && value === null) { this.value = value; } } @@ -753,7 +751,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan * @docs-private */ override writeValue(value: any): void { - if (this._isControlInitialized) { + if (this._isControlInitialized && value === null) { this.value = value; this._updateWidthInactive(); this._updateSibling(); From 3836658e3e7decf45944fef55ae72426bc86e1fa Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 24 May 2023 11:12:38 -0400 Subject: [PATCH 7/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider-input.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index 0d6544c5425a..d176e422e118 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -510,7 +510,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA * @docs-private */ writeValue(value: any): void { - if (this._isControlInitialized && value === null) { + if (this._isControlInitialized || value !== null) { this.value = value; } } @@ -751,7 +751,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan * @docs-private */ override writeValue(value: any): void { - if (this._isControlInitialized && value === null) { + if (this._isControlInitialized || value !== null) { this.value = value; this._updateWidthInactive(); this._updateSibling(); From c5139cc93294dd92489441137a61221915b3a815 Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 24 May 2023 11:24:28 -0400 Subject: [PATCH 8/9] fixup! fix(material/slider): handle ngModel initial null value --- tools/public_api_guard/material/slider.md | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/public_api_guard/material/slider.md b/tools/public_api_guard/material/slider.md index 68f4bfa071ea..a123d1bad943 100644 --- a/tools/public_api_guard/material/slider.md +++ b/tools/public_api_guard/material/slider.md @@ -218,6 +218,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA // (undocumented) _initValue(): void; _isActive: boolean; + protected _isControlInitialized: boolean; _isFocused: boolean; _knobRadius: number; get max(): number; From 4998bc6b078cf385d62127dfa4ed7986e6b2c8fc Mon Sep 17 00:00:00 2001 From: Wagner Maciel Date: Wed, 24 May 2023 12:38:26 -0400 Subject: [PATCH 9/9] fixup! fix(material/slider): handle ngModel initial null value --- src/material/slider/slider-input.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/material/slider/slider-input.ts b/src/material/slider/slider-input.ts index d176e422e118..f357d9dbabc8 100644 --- a/src/material/slider/slider-input.ts +++ b/src/material/slider/slider-input.ts @@ -432,7 +432,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA this.value = value; this.valueChange.emit(this.value); - this._onChangeFn!(this.value); + this._onChangeFn?.(this.value); this._slider._onValueChange(this); this._slider.step > 0 ? this._updateThumbUIByValue()