From 741b04a2df91fb5a419692c6aeb13b37134aa694 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Tue, 1 Jun 2021 07:52:48 +0200
Subject: [PATCH] fix(material/slider): don't interrupt pointer dragging when
 keyboard is pressed

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 d0bd33f853b4..a0a3b75866d8 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;