From 705f1448a967b6051afa8798bd0ac730cc2a03ff Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 7 Sep 2018 12:20:46 +0300 Subject: [PATCH] fix(slider): update value on mousedown instead of click Currently the slider updates its value on the `click` event or when the user drags for at least one pixel. This looks weird, because the thumb label shows up as soon as the user puts down their pointer, but the value updates only if they lift it or they start dragging. There will always be a value update, because the user can't move their pointer outside the element without either lifting it or triggering the drag event. Furthermore, the native `input type="range"` updates its value on `mousedown` as well and doing so makes the slider feel snappier. --- src/lib/slider/slider.spec.ts | 106 +++++++++++++++++----------------- src/lib/slider/slider.ts | 6 +- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index 7780fca2969a..12a0611e923d 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -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); }); @@ -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)'); @@ -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. @@ -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); }); @@ -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'); @@ -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); }); @@ -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. @@ -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 @@ -426,17 +426,17 @@ describe('MatSlider', () => { trackFillElement = 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. @@ -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. @@ -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); @@ -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(); @@ -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); @@ -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); @@ -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(); @@ -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); @@ -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); @@ -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); @@ -1155,39 +1155,39 @@ describe('MatSlider', () => { trackFillElement = 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)'); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); } /** @@ -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 { diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 780188303b3e..beb5488a070f 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -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()', @@ -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});