From 23dfbbb238b29fb3cc2bda680186cf0a9b8c40f1 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Thu, 3 Jun 2021 01:03:14 +0200
Subject: [PATCH] fix(material/slider): don't interrupt pointer dragging when
 keyboard is pressed (#22849)

The slider is currently set up to "slide" either with the keyboard or the pointer, but we don't dinstinguish between the two. This means that when a `keyup` fires, it'll interrupt pointer scrolling as well.

These changes add some logic to prevent the two from interfering with each other.

Fixes #22719.
---
 src/material/slider/slider.spec.ts          | 23 +++++++++++++++++++++
 src/material/slider/slider.ts               | 21 +++++++++++--------
 tools/public_api_guard/material/slider.d.ts |  2 +-
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/src/material/slider/slider.spec.ts b/src/material/slider/slider.spec.ts
index ba5a09b19a45..1c95fd6a294a 100644
--- a/src/material/slider/slider.spec.ts
+++ b/src/material/slider/slider.spec.ts
@@ -9,6 +9,7 @@ import {
   PAGE_UP,
   RIGHT_ARROW,
   UP_ARROW,
+  A,
 } from '@angular/cdk/keycodes';
 import {
   createMouseEvent,
@@ -139,6 +140,28 @@ describe('MatSlider', () => {
       expect(sliderNativeElement.classList).not.toContain('mat-slider-sliding');
     });
 
+    it('should not interrupt sliding by pressing a key', () => {
+      expect(sliderNativeElement.classList).not.toContain('mat-slider-sliding');
+
+      dispatchSlideStartEvent(sliderNativeElement, 0);
+      fixture.detectChanges();
+
+      expect(sliderNativeElement.classList).toContain('mat-slider-sliding');
+
+      // Any key code will do here. Use A since it isn't associated with other actions.
+      dispatchKeyboardEvent(sliderNativeElement, 'keydown', A);
+      fixture.detectChanges();
+      dispatchKeyboardEvent(sliderNativeElement, 'keyup', A);
+      fixture.detectChanges();
+
+      expect(sliderNativeElement.classList).toContain('mat-slider-sliding');
+
+      dispatchSlideEndEvent(sliderNativeElement, 0.34);
+      fixture.detectChanges();
+
+      expect(sliderNativeElement.classList).not.toContain('mat-slider-sliding');
+    });
+
     it('should stop dragging if the page loses focus', () => {
       const classlist = sliderNativeElement.classList;
 
diff --git a/src/material/slider/slider.ts b/src/material/slider/slider.ts
index ea4bfa1e1f36..2cedf486bf80 100644
--- a/src/material/slider/slider.ts
+++ b/src/material/slider/slider.ts
@@ -326,10 +326,10 @@ export class MatSlider extends _MatSliderBase
   private _percent: number = 0;
 
   /**
-   * Whether or not the thumb is sliding.
+   * Whether or not the thumb is sliding and what the user is using to slide it with.
    * Used to determine if there should be a transition for the thumb and fill track.
    */
-  _isSliding: boolean = false;
+  _isSliding: 'keyboard' | 'pointer' | null = null;
 
   /**
    * Whether or not the slider is active (clicked or sliding).
@@ -569,7 +569,8 @@ export class MatSlider extends _MatSliderBase
   }
 
   _onKeydown(event: KeyboardEvent) {
-    if (this.disabled || hasModifierKey(event)) {
+    if (this.disabled || hasModifierKey(event) ||
+        (this._isSliding && this._isSliding !== 'keyboard')) {
       return;
     }
 
@@ -619,12 +620,14 @@ export class MatSlider extends _MatSliderBase
       this._emitChangeEvent();
     }
 
-    this._isSliding = true;
+    this._isSliding = 'keyboard';
     event.preventDefault();
   }
 
   _onKeyup() {
-    this._isSliding = false;
+    if (this._isSliding === 'keyboard') {
+      this._isSliding = null;
+    }
   }
 
   /** Called when the user has put their pointer down on the slider. */
@@ -642,7 +645,7 @@ export class MatSlider extends _MatSliderBase
 
       if (pointerPosition) {
         const oldValue = this.value;
-        this._isSliding = true;
+        this._isSliding = 'pointer';
         this._lastPointerEvent = event;
         event.preventDefault();
         this._focusHostElement();
@@ -665,7 +668,7 @@ export class MatSlider extends _MatSliderBase
    * starting to drag. Bound on the document level.
    */
   private _pointerMove = (event: TouchEvent | MouseEvent) => {
-    if (this._isSliding) {
+    if (this._isSliding === 'pointer') {
       const pointerPosition = getPointerPositionOnPage(event, this._touchId);
 
       if (pointerPosition) {
@@ -685,14 +688,14 @@ export class MatSlider extends _MatSliderBase
 
   /** Called when the user has lifted their pointer. Bound on the document level. */
   private _pointerUp = (event: TouchEvent | MouseEvent) => {
-    if (this._isSliding) {
+    if (this._isSliding === 'pointer') {
       if (!isTouchEvent(event) || typeof this._touchId !== 'number' ||
           // Note that we use `changedTouches`, rather than `touches` because it
           // seems like in most cases `touches` is empty for `touchend` events.
           findMatchingTouch(event.changedTouches, this._touchId)) {
         event.preventDefault();
         this._removeGlobalEvents();
-        this._isSliding = false;
+        this._isSliding = null;
         this._touchId = undefined;
 
         if (this._valueOnSlideStart != this.value && !this.disabled) {
diff --git a/tools/public_api_guard/material/slider.d.ts b/tools/public_api_guard/material/slider.d.ts
index ecc22595b964..df115a5d08dd 100644
--- a/tools/public_api_guard/material/slider.d.ts
+++ b/tools/public_api_guard/material/slider.d.ts
@@ -4,7 +4,7 @@ export declare class MatSlider extends _MatSliderBase implements ControlValueAcc
     _animationMode?: string | undefined;
     protected _document: Document;
     _isActive: boolean;
-    _isSliding: boolean;
+    _isSliding: 'keyboard' | 'pointer' | null;
     readonly change: EventEmitter<MatSliderChange>;
     get displayValue(): string | number;
     displayWith: (value: number) => string | number;