Skip to content

fix(slider): update value on mousedown instead of click #13020

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.

Sign up for GitHub

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
Oct 19, 2018
Merged

fix(slider): update value on mousedown instead of click #13020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
106 changes: 53 additions & 53 deletions src/lib/slider/slider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ describe('MatSlider', () => {
expect(sliderInstance.max).toBe(100);
});

it('should update the value on a click', () => {
it('should update the value on mousedown', () => {
expect(sliderInstance.value).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.19);
dispatchMousedownEventSequence(sliderNativeElement, 0.19);

expect(sliderInstance.value).toBe(19);
});
Expand Down Expand Up @@ -92,10 +92,10 @@ describe('MatSlider', () => {
expect(sliderInstance.value).toBe(100);
});

it('should update the track fill on click', () => {
it('should update the track fill on mousedown', () => {
expect(trackFillElement.style.transform).toContain('scale3d(0, 1, 1)');

dispatchClickEventSequence(sliderNativeElement, 0.39);
dispatchMousedownEventSequence(sliderNativeElement, 0.39);
fixture.detectChanges();

expect(trackFillElement.style.transform).toContain('scale3d(0.39, 1, 1)');
Expand Down Expand Up @@ -178,7 +178,7 @@ describe('MatSlider', () => {
});

it('should not have thumb gap when not at min value', () => {
dispatchClickEventSequence(sliderNativeElement, 1);
dispatchMousedownEventSequence(sliderNativeElement, 1);
fixture.detectChanges();

// Some browsers use '0' and some use '0px', so leave off the closing paren.
Expand Down Expand Up @@ -223,10 +223,10 @@ describe('MatSlider', () => {
expect(sliderInstance.disabled).toBeTruthy();
});

it('should not change the value on click when disabled', () => {
it('should not change the value on mousedown when disabled', () => {
expect(sliderInstance.value).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.63);
dispatchMousedownEventSequence(sliderNativeElement, 0.63);

expect(sliderInstance.value).toBe(0);
});
Expand All @@ -248,10 +248,10 @@ describe('MatSlider', () => {
expect(onChangeSpy).toHaveBeenCalledTimes(0);
});

it('should not add the mat-slider-active class on click when disabled', () => {
it('should not add the mat-slider-active class on mousedown when disabled', () => {
expect(sliderNativeElement.classList).not.toContain('mat-slider-active');

dispatchClickEventSequence(sliderNativeElement, 0.43);
dispatchMousedownEventSequence(sliderNativeElement, 0.43);
fixture.detectChanges();

expect(sliderNativeElement.classList).not.toContain('mat-slider-active');
Expand Down Expand Up @@ -305,12 +305,12 @@ describe('MatSlider', () => {
expect(sliderInstance.max).toBe(6);
});

it('should set the correct value on click', () => {
dispatchClickEventSequence(sliderNativeElement, 0.09);
it('should set the correct value on mousedown', () => {
dispatchMousedownEventSequence(sliderNativeElement, 0.09);
fixture.detectChanges();

// Computed by multiplying the difference between the min and the max by the percentage from
// the click and adding that to the minimum.
// the mousedown and adding that to the minimum.
let value = Math.round(4 + (0.09 * (6 - 4)));
expect(sliderInstance.value).toBe(value);
});
Expand All @@ -320,13 +320,13 @@ describe('MatSlider', () => {
fixture.detectChanges();

// Computed by multiplying the difference between the min and the max by the percentage from
// the click and adding that to the minimum.
// the mousedown and adding that to the minimum.
let value = Math.round(4 + (0.62 * (6 - 4)));
expect(sliderInstance.value).toBe(value);
});

it('should snap the fill to the nearest value on click', () => {
dispatchClickEventSequence(sliderNativeElement, 0.68);
it('should snap the fill to the nearest value on mousedown', () => {
dispatchMousedownEventSequence(sliderNativeElement, 0.68);
fixture.detectChanges();

// The closest snap is halfway on the slider.
Expand Down Expand Up @@ -392,8 +392,8 @@ describe('MatSlider', () => {
expect(sliderInstance.value).toBe(26);
});

it('should set the correct value on click', () => {
dispatchClickEventSequence(sliderNativeElement, 0.92);
it('should set the correct value on mousedown', () => {
dispatchMousedownEventSequence(sliderNativeElement, 0.92);
fixture.detectChanges();

// On a slider with default max and min the value should be approximately equal to the
Expand Down Expand Up @@ -426,17 +426,17 @@ describe('MatSlider', () => {
trackFillElement = <HTMLElement>sliderNativeElement.querySelector('.mat-slider-track-fill');
});

it('should set the correct step value on click', () => {
it('should set the correct step value on mousedown', () => {
expect(sliderInstance.value).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.13);
dispatchMousedownEventSequence(sliderNativeElement, 0.13);
fixture.detectChanges();

expect(sliderInstance.value).toBe(25);
});

it('should snap the fill to a step on click', () => {
dispatchClickEventSequence(sliderNativeElement, 0.66);
it('should snap the fill to a step on mousedown', () => {
dispatchMousedownEventSequence(sliderNativeElement, 0.66);
fixture.detectChanges();

// The closest step is at 75% of the slider.
Expand Down Expand Up @@ -596,10 +596,10 @@ describe('MatSlider', () => {
expect(sliderNativeElement.classList).toContain('mat-slider-thumb-label-showing');
});

it('should update the thumb label text on click', () => {
it('should update the thumb label text on mousedown', () => {
expect(thumbLabelTextElement.textContent).toBe('0');

dispatchClickEventSequence(sliderNativeElement, 0.13);
dispatchMousedownEventSequence(sliderNativeElement, 0.13);
fixture.detectChanges();

// The thumb label text is set to the slider's value. These should always be the same.
Expand Down Expand Up @@ -757,10 +757,10 @@ describe('MatSlider', () => {
sliderNativeElement = sliderDebugElement.nativeElement;
});

it('should emit change on click', () => {
it('should emit change on mousedown', () => {
expect(testComponent.onChange).not.toHaveBeenCalled();

dispatchClickEventSequence(sliderNativeElement, 0.2);
dispatchMousedownEventSequence(sliderNativeElement, 0.2);
fixture.detectChanges();

expect(testComponent.onChange).toHaveBeenCalledTimes(1);
Expand All @@ -778,7 +778,7 @@ describe('MatSlider', () => {
it('should not emit multiple changes for same value', () => {
expect(testComponent.onChange).not.toHaveBeenCalled();

dispatchClickEventSequence(sliderNativeElement, 0.6);
dispatchMousedownEventSequence(sliderNativeElement, 0.6);
dispatchSlideEventSequence(sliderNativeElement, 0.6, 0.6, gestureConfig);
fixture.detectChanges();

Expand All @@ -790,7 +790,7 @@ describe('MatSlider', () => {
expect(testComponent.onChange).not.toHaveBeenCalled();
expect(testComponent.onInput).not.toHaveBeenCalled();

dispatchClickEventSequence(sliderNativeElement, 0.2);
dispatchMousedownEventSequence(sliderNativeElement, 0.2);
fixture.detectChanges();

expect(testComponent.onChange).toHaveBeenCalledTimes(1);
Expand All @@ -802,7 +802,7 @@ describe('MatSlider', () => {
expect(testComponent.onChange).toHaveBeenCalledTimes(1);
expect(testComponent.onInput).toHaveBeenCalledTimes(1);

dispatchClickEventSequence(sliderNativeElement, 0.2);
dispatchMousedownEventSequence(sliderNativeElement, 0.2);
fixture.detectChanges();

expect(testComponent.onChange).toHaveBeenCalledTimes(2);
Expand Down Expand Up @@ -846,7 +846,7 @@ describe('MatSlider', () => {
it('should emit an input event when clicking', () => {
expect(testComponent.onChange).not.toHaveBeenCalled();

dispatchClickEventSequence(sliderNativeElement, 0.75);
dispatchMousedownEventSequence(sliderNativeElement, 0.75);

fixture.detectChanges();

Expand Down Expand Up @@ -1017,7 +1017,7 @@ describe('MatSlider', () => {
testComponent.invert = true;
fixture.detectChanges();

dispatchClickEventSequence(sliderNativeElement, 0.3);
dispatchMousedownEventSequence(sliderNativeElement, 0.3);
fixture.detectChanges();

expect(sliderInstance.value).toBe(70);
Expand All @@ -1027,7 +1027,7 @@ describe('MatSlider', () => {
testComponent.dir = 'rtl';
fixture.detectChanges();

dispatchClickEventSequence(sliderNativeElement, 0.3);
dispatchMousedownEventSequence(sliderNativeElement, 0.3);
fixture.detectChanges();

expect(sliderInstance.value).toBe(70);
Expand All @@ -1038,7 +1038,7 @@ describe('MatSlider', () => {
testComponent.invert = true;
fixture.detectChanges();

dispatchClickEventSequence(sliderNativeElement, 0.3);
dispatchMousedownEventSequence(sliderNativeElement, 0.3);
fixture.detectChanges();

expect(sliderInstance.value).toBe(30);
Expand Down Expand Up @@ -1155,39 +1155,39 @@ describe('MatSlider', () => {
trackFillElement = <HTMLElement>sliderNativeElement.querySelector('.mat-slider-track-fill');
});

it('updates value on click', () => {
dispatchClickEventSequence(sliderNativeElement, 0.3);
it('updates value on mousedown', () => {
dispatchMousedownEventSequence(sliderNativeElement, 0.3);
fixture.detectChanges();

expect(sliderInstance.value).toBe(70);
});

it('updates value on click in inverted mode', () => {
it('updates value on mousedown in inverted mode', () => {
testComponent.invert = true;
fixture.detectChanges();

dispatchClickEventSequence(sliderNativeElement, 0.3);
dispatchMousedownEventSequence(sliderNativeElement, 0.3);
fixture.detectChanges();

expect(sliderInstance.value).toBe(30);
});

it('should update the track fill on click', () => {
it('should update the track fill on mousedown', () => {
expect(trackFillElement.style.transform).toContain('scale3d(1, 0, 1)');

dispatchClickEventSequence(sliderNativeElement, 0.39);
dispatchMousedownEventSequence(sliderNativeElement, 0.39);
fixture.detectChanges();

expect(trackFillElement.style.transform).toContain('scale3d(1, 0.61, 1)');
});

it('should update the track fill on click in inverted mode', () => {
it('should update the track fill on mousedown in inverted mode', () => {
testComponent.invert = true;
fixture.detectChanges();

expect(trackFillElement.style.transform).toContain('scale3d(1, 0, 1)');

dispatchClickEventSequence(sliderNativeElement, 0.39);
dispatchMousedownEventSequence(sliderNativeElement, 0.39);
fixture.detectChanges();

expect(trackFillElement.style.transform).toContain('scale3d(1, 0.39, 1)');
Expand Down Expand Up @@ -1241,10 +1241,10 @@ describe('MatSlider', () => {
sliderNativeElement = sliderDebugElement.nativeElement;
});

it('should update the model on click', () => {
it('should update the model on mousedown', () => {
expect(testComponent.val).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.76);
dispatchMousedownEventSequence(sliderNativeElement, 0.76);
fixture.detectChanges();

expect(testComponent.val).toBe(76);
Expand Down Expand Up @@ -1313,10 +1313,10 @@ describe('MatSlider', () => {
expect(testComponent.control.value).toBe(0);
});

it('should update the control on click', () => {
it('should update the control on mousedown', () => {
expect(testComponent.control.value).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.76);
dispatchMousedownEventSequence(sliderNativeElement, 0.76);
fixture.detectChanges();

expect(testComponent.control.value).toBe(76);
Expand Down Expand Up @@ -1368,7 +1368,7 @@ describe('MatSlider', () => {

// After changing the value, the control should become dirty (not pristine),
// but remain untouched.
dispatchClickEventSequence(sliderNativeElement, 0.5);
dispatchMousedownEventSequence(sliderNativeElement, 0.5);
fixture.detectChanges();

expect(sliderControl.valid).toBe(true);
Expand Down Expand Up @@ -1404,7 +1404,7 @@ describe('MatSlider', () => {
expect(testComponent.value).toBe(0);
expect(testComponent.slider.value).toBe(0);

dispatchClickEventSequence(sliderNativeElement, 0.1);
dispatchMousedownEventSequence(sliderNativeElement, 0.1);
fixture.detectChanges();

expect(testComponent.value).toBe(10);
Expand Down Expand Up @@ -1594,20 +1594,20 @@ class SliderWithTwoWayBinding {
}

/**
* Dispatches a click event sequence (consisting of moueseenter, click) from an element.
* Note: The mouse event truncates the position for the click.
* Dispatches a mousedown event sequence (consisting of moueseenter, mousedown) from an element.
* Note: The mouse event truncates the position for the event.
* @param sliderElement The mat-slider element from which the event will be dispatched.
* @param percentage The percentage of the slider where the click should occur. Used to find the
* physical location of the click.
* @param percentage The percentage of the slider where the event should occur. Used to find the
* physical location of the pointer.
*/
function dispatchClickEventSequence(sliderElement: HTMLElement, percentage: number): void {
function dispatchMousedownEventSequence(sliderElement: HTMLElement, percentage: number): void {
let trackElement = sliderElement.querySelector('.mat-slider-wrapper')!;
let dimensions = trackElement.getBoundingClientRect();
let x = dimensions.left + (dimensions.width * percentage);
let y = dimensions.top + (dimensions.height * percentage);

dispatchMouseenterEvent(sliderElement);
dispatchMouseEvent(sliderElement, 'click', x, y);
dispatchMouseEvent(sliderElement, 'mousedown', x, y);
}

/**
Expand Down Expand Up @@ -1687,7 +1687,7 @@ function dispatchSlideEndEvent(sliderElement: HTMLElement, percent: number,

/**
* Dispatches a mouseenter event from an element.
* Note: The mouse event truncates the position for the click.
* Note: The mouse event truncates the position for the event.
* @param element The element from which the event will be dispatched.
*/
function dispatchMouseenterEvent(element: HTMLElement): void {
Expand Down
6 changes: 3 additions & 3 deletions src/lib/slider/slider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const _MatSliderMixinBase:
host: {
'(focus)': '_onFocus()',
'(blur)': '_onBlur()',
'(click)': '_onClick($event)',
'(mousedown)': '_onMousedown($event)',
'(keydown)': '_onKeydown($event)',
'(keyup)': '_onKeyup()',
'(mouseenter)': '_onMouseenter()',
Expand Down Expand Up @@ -500,12 +500,12 @@ export class MatSlider extends _MatSliderMixinBase
this._updateTickIntervalPercent();
}

_onClick(event: MouseEvent) {
_onMousedown(event: MouseEvent) {
if (this.disabled) {
return;
}

let oldValue = this.value;
const oldValue = this.value;
this._isSliding = false;
this._focusHostElement();
this._updateValueFromPosition({x: event.clientX, y: event.clientY});
Expand Down