Skip to content

fix(material/slider): don't interrupt pointer dragging when keyboard is pressed #22849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/material/slider/slider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
PAGE_UP,
RIGHT_ARROW,
UP_ARROW,
A,
} from '@angular/cdk/keycodes';
import {
createMouseEvent,
Expand Down Expand Up @@ -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;

Expand Down
21 changes: 12 additions & 9 deletions src/material/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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. */
Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/material/slider.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down