From b8831c22c5d5bc63dd7a6f42b2aa84af2e611253 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Thu, 24 Jun 2021 18:11:59 +0200
Subject: [PATCH] fix(material-experimental/mdc-slider): event handling fixes
 and cleanup

* Fixes that the `mouseenter` and `mouseleave` handlers on the slider were leaking. It was `Function.bind` returns a new function and we weren't passing the same function in when the component is destroyed.
* Fixes that slider was calling `unsubscribe` on subjects that belong to the slider thumbs. `Subject.unsubscribe` will drop all subscriptions to the observable, including ones that come from the outside. The correct thing to do is to either complete the observables or save a reference to the `Subscription` and call `unsubscribe` on it.
* Removes the `*Ctor` interfaces from the mixin class since it isn't necessary anymore.
---
 .../mdc-slider/slider.ts                      | 43 +++++++++----------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/src/material-experimental/mdc-slider/slider.ts b/src/material-experimental/mdc-slider/slider.ts
index 925e523841dd..7040e869c1a8 100644
--- a/src/material-experimental/mdc-slider/slider.ts
+++ b/src/material-experimental/mdc-slider/slider.ts
@@ -39,9 +39,7 @@ import {
 } from '@angular/core';
 import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
 import {
-  CanColorCtor,
   CanDisableRipple,
-  CanDisableRippleCtor,
   MatRipple,
   MAT_RIPPLE_GLOBAL_OPTIONS,
   mixinColor,
@@ -132,30 +130,27 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy {
     this._ripple.radius = 24;
     this._sliderInput = this._slider._getInput(this.thumbPosition);
 
-    this._sliderInput.dragStart.subscribe((e: MatSliderDragEvent) => this._onDragStart(e));
-    this._sliderInput.dragEnd.subscribe((e: MatSliderDragEvent) => this._onDragEnd(e));
+    // Note that we don't unsubscribe from these, because they're complete on destroy.
+    this._sliderInput.dragStart.subscribe(event => this._onDragStart(event));
+    this._sliderInput.dragEnd.subscribe(event => this._onDragEnd(event));
 
     this._sliderInput._focus.subscribe(() => this._onFocus());
     this._sliderInput._blur.subscribe(() => this._onBlur());
 
     // These two listeners don't update any data bindings so we bind them
-    // outside of the NgZone to pervent angular from needlessly running change detection.
+    // outside of the NgZone to prevent Angular from needlessly running change detection.
     this._ngZone.runOutsideAngular(() => {
-      this._elementRef.nativeElement.addEventListener('mouseenter', this._onMouseEnter.bind(this));
-      this._elementRef.nativeElement.addEventListener('mouseleave', this._onMouseLeave.bind(this));
+      this._elementRef.nativeElement.addEventListener('mouseenter', this._onMouseEnter);
+      this._elementRef.nativeElement.addEventListener('mouseleave', this._onMouseLeave);
     });
   }
 
   ngOnDestroy() {
-    this._sliderInput.dragStart.unsubscribe();
-    this._sliderInput.dragEnd.unsubscribe();
-    this._sliderInput._focus.unsubscribe();
-    this._sliderInput._blur.unsubscribe();
     this._elementRef.nativeElement.removeEventListener('mouseenter', this._onMouseEnter);
     this._elementRef.nativeElement.removeEventListener('mouseleave', this._onMouseLeave);
   }
 
-  private _onMouseEnter(): void {
+  private _onMouseEnter = (): void => {
     this._isHovered = true;
     // We don't want to show the hover ripple on top of the focus ripple.
     // This can happen if the user tabs to a thumb and then the user moves their cursor over it.
@@ -164,7 +159,7 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy {
     }
   }
 
-  private _onMouseLeave(): void {
+  private _onMouseLeave = (): void => {
     this._isHovered = false;
     this._hoverRippleRef?.fadeOut();
   }
@@ -279,7 +274,7 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy {
     multi: true
   }],
 })
-export class MatSliderThumb implements AfterViewInit, ControlValueAccessor, OnInit {
+export class MatSliderThumb implements AfterViewInit, ControlValueAccessor, OnInit, OnDestroy {
 
   // ** IMPORTANT NOTE **
   //
@@ -315,7 +310,7 @@ export class MatSliderThumb implements AfterViewInit, ControlValueAccessor, OnIn
    * to facilitate the two-way binding for the `value` input.
    * @docs-private
    */
-   @Output() readonly valueChange: EventEmitter<number> = new EventEmitter<number>();
+  @Output() readonly valueChange: EventEmitter<number> = new EventEmitter<number>();
 
   /** Event emitted when the slider thumb starts being dragged. */
   @Output() readonly dragStart: EventEmitter<MatSliderDragEvent>
@@ -386,6 +381,14 @@ export class MatSliderThumb implements AfterViewInit, ControlValueAccessor, OnIn
     }
   }
 
+  ngOnDestroy() {
+    this.dragStart.complete();
+    this.dragEnd.complete();
+    this._focus.complete();
+    this._blur.complete();
+    this.valueChange.complete();
+  }
+
   _onBlur(): void {
     this._onTouched();
     this._blur.emit();
@@ -512,15 +515,9 @@ export class MatSliderThumb implements AfterViewInit, ControlValueAccessor, OnIn
 }
 
 // Boilerplate for applying mixins to MatSlider.
-/** @docs-private */
-class MatSliderBase {
+const _MatSliderMixinBase = mixinColor(mixinDisableRipple(class {
   constructor(public _elementRef: ElementRef<HTMLElement>) {}
-}
-const _MatSliderMixinBase:
-    CanColorCtor &
-    CanDisableRippleCtor &
-    typeof MatSliderBase =
-        mixinColor(mixinDisableRipple(MatSliderBase), 'primary');
+}), 'primary');
 
 /**
  * Allows users to select from a range of values by moving the slider thumb. It is similar in