From 1d6788eae494cd2f3b63809c3bfa7b373f2c1160 Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Mon, 1 Mar 2021 12:37:33 -0800 Subject: [PATCH 1/7] feat(material-experimental/mdc-slider): implement some basic unit tests * implement unit tests for the standard slider, standard range slider, and for the slider ripple states * add mdc-slider theme to all-theme * make MatSliderVisualThumb ripple refs public so their states can be checked & tested * make MatSlider child component and html element getters default thumbPosition to the end thumb --- .../mdc-slider/BUILD.bazel | 2 + .../mdc-slider/slider.spec.ts | 333 +++++++++++++++++- .../mdc-slider/slider.ts | 19 +- .../mdc-theming/_all-theme.scss | 2 + 4 files changed, 341 insertions(+), 15 deletions(-) diff --git a/src/material-experimental/mdc-slider/BUILD.bazel b/src/material-experimental/mdc-slider/BUILD.bazel index 5aeb7db3c082..cb14d0a3ba2c 100644 --- a/src/material-experimental/mdc-slider/BUILD.bazel +++ b/src/material-experimental/mdc-slider/BUILD.bazel @@ -75,8 +75,10 @@ ng_test_library( "//src/cdk/keycodes", "//src/cdk/platform", "//src/cdk/testing/private", + "//src/material/core", "@npm//@angular/forms", "@npm//@angular/platform-browser", + "@npm//@material/slider", ], ) diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index e702f5243e95..92ed2274cb91 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -6,14 +6,337 @@ * found in the LICENSE file at https://angular.io/license */ +import {dispatchMouseEvent, dispatchPointerEvent} from '@angular/cdk/testing/private'; +import {Component, DebugElement, Type} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {RippleRef, RippleState} from '@angular/material/core'; +import {By} from '@angular/platform-browser'; +import {Thumb} from '@material/slider'; +import {MatSliderModule} from './module'; +import {MatSlider, MatSliderThumb, MatSliderVisualThumb} from './slider'; -/* tslint:disable-next-line:no-unused-variable */ -import {MatSlider} from './index'; +describe('MDC-based MatSlider' , () => { + beforeAll(() => { + // Mock #setPointerCapture as it throws errors on pointerdown without a real pointerId. + spyOn(Element.prototype, 'setPointerCapture'); + }); -// TODO(wagnermaciel): Implement this in a separate PR + function createComponent(component: Type): ComponentFixture { + TestBed.configureTestingModule({ + imports: [MatSliderModule], + declarations: [component], + }).compileComponents(); + return TestBed.createComponent(component); + } -describe('MDC-based MatSlider' , () => { describe('standard slider', () => { - it('does nothing yet', () => {}); + let fixture: ComponentFixture; + let sliderDebugElement: DebugElement; + let sliderInstance: MatSlider; + let inputInstance: MatSliderThumb; + + beforeEach(() => { + fixture = createComponent(StandardSlider); + fixture.detectChanges(); + sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); + sliderInstance = sliderDebugElement.componentInstance; + inputInstance = sliderInstance._getInput(Thumb.END); + }); + + beforeEach(done => { + fixture.whenStable().then(() => done()); + }); + + it('should set the default values', () => { + expect(inputInstance.value).toBe(0); + expect(sliderInstance.min).toBe(0); + expect(sliderInstance.max).toBe(100); + }); + + it('should update the value on mousedown', () => { + setValueByClick(sliderInstance, 19); + expect(inputInstance.value).toBe(19); + }); + + it('should update the value on a slide', () => { + slideToValue(sliderInstance, 77); + expect(inputInstance.value).toBe(77); + }); + + it('should set the value as min when sliding before the track', () => { + slideToValue(sliderInstance, -1); + expect(inputInstance.value).toBe(0); + }); + + it('should set the value as max when sliding past the track', () => { + slideToValue(sliderInstance, 101); + expect(inputInstance.value).toBe(100); + }); + + it('should focus the slider input when clicking on the slider', () => { + expect(document.activeElement).not.toBe(inputInstance._hostElement); + setValueByClick(sliderInstance, 0); + expect(document.activeElement).toBe(inputInstance._hostElement); + }); + }); + + describe('standard range slider', () => { + let fixture: ComponentFixture; + let sliderDebugElement: DebugElement; + let sliderInstance: MatSlider; + let startInputInstance: MatSliderThumb; + let endInputInstance: MatSliderThumb; + + beforeEach(() => { + fixture = createComponent(StandardRangeSlider); + fixture.detectChanges(); + sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); + sliderInstance = sliderDebugElement.componentInstance; + startInputInstance = sliderInstance._getInput(Thumb.START); + endInputInstance = sliderInstance._getInput(Thumb.END); + }); + + beforeEach(done => { + fixture.whenStable().then(() => done()); + }); + + it('should set the default values', () => { + expect(startInputInstance.value).toBe(0); + expect(endInputInstance.value).toBe(100); + expect(sliderInstance.min).toBe(0); + expect(sliderInstance.max).toBe(100); + }); + + it('should update the start value on a slide', () => { + slideToValue(sliderInstance, 19, Thumb.START); + expect(startInputInstance.value).toBe(19); + }); + + it('should update the end value on a slide', () => { + slideToValue(sliderInstance, 27, Thumb.END); + expect(endInputInstance.value).toBe(27); + }); + + it('should update the start value on mousedown behind the start thumb', () => { + sliderInstance._setValue(19, Thumb.START); + setValueByClick(sliderInstance, 12); + expect(startInputInstance.value).toBe(12); + }); + + it('should update the end value on mousedown in front of the end thumb', () => { + sliderInstance._setValue(27, Thumb.END); + setValueByClick(sliderInstance, 55); + expect(endInputInstance.value).toBe(55); + }); + + it('should set the start value as min when sliding before the track', () => { + slideToValue(sliderInstance, -1, Thumb.START); + expect(startInputInstance.value).toBe(0); + }); + + it('should set the end value as max when sliding past the track', () => { + slideToValue(sliderInstance, 101, Thumb.START); + expect(startInputInstance.value).toBe(100); + }); + + it('should not let the start thumb slide past the end thumb', () => { + sliderInstance._setValue(50, Thumb.END); + slideToValue(sliderInstance, 75, Thumb.START); + expect(startInputInstance.value).toBe(50); + }); + + it('should not let the end thumb slide before the start thumb', () => { + sliderInstance._setValue(50, Thumb.START); + slideToValue(sliderInstance, 25, Thumb.END); + expect(startInputInstance.value).toBe(50); + }); + }); + + describe('ripple states', () => { + let fixture: ComponentFixture; + let inputInstance: MatSliderThumb; + let thumbInstance: MatSliderVisualThumb; + let thumbElement: HTMLElement; + let thumbX: number; + let thumbY: number; + + beforeEach(() => { + fixture = createComponent(StandardSlider); + fixture.detectChanges(); + const sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); + const sliderInstance = sliderDebugElement.componentInstance; + inputInstance = sliderInstance._getInput(Thumb.END); + thumbInstance = sliderInstance._getThumb(); + thumbElement = thumbInstance._getHostElement(); + }); + + beforeEach(done => { + fixture.whenStable().then(() => done()); + }); + + beforeEach(() => { + const thumbDimensions = thumbElement.getBoundingClientRect(); + thumbX = thumbDimensions.left - (thumbDimensions.width / 2); + thumbY = thumbDimensions.top - (thumbDimensions.height / 2); + }); + + function isRippleVisible(rippleRef: RippleRef) { + return rippleRef?.state === RippleState.FADING_IN + || rippleRef?.state === RippleState.VISIBLE; + } + + function blur() { + inputInstance._hostElement.blur(); + } + + function mouseenter() { + dispatchMouseEvent(thumbElement, 'mouseenter', thumbX, thumbY); + } + + function mouseleave() { + dispatchMouseEvent(thumbElement, 'mouseleave', thumbX, thumbY); + } + + function pointerdown() { + dispatchPointerEvent(thumbElement, 'pointerdown', thumbX, thumbY); + } + + function pointerup() { + dispatchPointerEvent(thumbElement, 'pointerup', thumbX, thumbY); + } + + it('should show the hover ripple on mouseenter', () => { + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); + mouseenter(); + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true); + }); + + it('should hide the hover ripple on mouseleave', () => { + mouseenter(); + mouseleave(); + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); + }); + + it('should show the focus ripple on pointerdown', () => { + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); + pointerdown(); + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); + }); + + it('should continue to show the focus ripple on pointerup', () => { + pointerdown(); + pointerup(); + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); + }); + + it('should hide the focus ripple on blur', () => { + pointerdown(); + pointerup(); + blur(); + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); + }); + + it('should show the active ripple on pointerdown', () => { + expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false); + pointerdown(); + expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(true); + }); + + it('should hide the active ripple on pointerup', () => { + pointerdown(); + pointerup(); + expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false); + }); + + // Edge cases. + + it('should not show the hover ripple if the thumb is already focused', () => { + pointerdown(); + mouseenter(); + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); + }); + + it('should hide the hover ripple if the thumb is focused', () => { + mouseenter(); + pointerdown(); + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); + }); + + it('should not hide the focus ripple if the thumb is pressed', () => { + pointerdown(); + blur(); + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); + }); + + it('should not hide the hover ripple on blur if the cursor is thumb being hovered', () => { + mouseenter(); + pointerdown(); + pointerup(); + blur(); + expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true); + }); + + it('should hide the focus ripple on drag end if the thumb already lost focus', () => { + pointerdown(); + blur(); + pointerup(); + expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); + }); }); }); + + +@Component({ + template: ` + + + + `, +}) +class StandardSlider {} + +@Component({ + template: ` + + + + + `, +}) +class StandardRangeSlider {} + +/** Clicks on the MatSlider at the coordinates corresponding to the given value. */ +function setValueByClick(slider: MatSlider, value: number) { + const {min, max} = slider; + const percent = (value - min) / (max - min); + + const sliderElement = slider._elementRef.nativeElement; + const {top, left, width, height} = sliderElement.getBoundingClientRect(); + const x = left + (width * percent); + const y = top + (height / 2); + + dispatchPointerEvent(sliderElement, 'pointerdown', x, y); + dispatchPointerEvent(sliderElement, 'pointerup', x, y); +} + +/** Slides the MatSlider's thumb to the given value. */ +function slideToValue(slider: MatSlider, value: number, thumbPosition: Thumb = Thumb.END) { + const {min, max} = slider; + const percent = (value - min) / (max - min); + + const sliderElement = slider._elementRef.nativeElement; + const thumbElement = slider._getThumbElement(thumbPosition); + + const sliderDimensions = sliderElement.getBoundingClientRect(); + let thumbDimensions = thumbElement.getBoundingClientRect(); + + const startX = thumbDimensions.left + (thumbDimensions.width / 2); + const startY = thumbDimensions.top + (thumbDimensions.height / 2); + + const endX = sliderDimensions.left + (sliderDimensions.width * percent); + const endY = sliderDimensions.top + (sliderDimensions.height / 2); + + dispatchPointerEvent(sliderElement, 'pointerdown', startX, startY); + dispatchPointerEvent(sliderElement, 'pointermove', endX, endY); + dispatchPointerEvent(sliderElement, 'pointerup', endX, endY); +} diff --git a/src/material-experimental/mdc-slider/slider.ts b/src/material-experimental/mdc-slider/slider.ts index 4d3c96343a08..fd1c443a2aa5 100644 --- a/src/material-experimental/mdc-slider/slider.ts +++ b/src/material-experimental/mdc-slider/slider.ts @@ -94,13 +94,13 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy { private _sliderInput: MatSliderThumb; /** The RippleRef for the slider thumbs hover state. */ - private _hoverRippleRef: RippleRef; + _hoverRippleRef: RippleRef; /** The RippleRef for the slider thumbs focus state. */ - private _focusRippleRef: RippleRef; + _focusRippleRef: RippleRef; /** The RippleRef for the slider thumbs active state. */ - private _activeRippleRef: RippleRef; + _activeRippleRef: RippleRef; /** Whether the slider thumb is currently being pressed. */ private _isActive: boolean = false; @@ -332,8 +332,7 @@ export class MatSliderThumb implements AfterViewInit, ControlValueAccessor { constructor( @Inject(DOCUMENT) document: any, private readonly _slider: MatSlider, - private readonly _elementRef: ElementRef, - ) { + private readonly _elementRef: ElementRef) { this._document = document; this._hostElement = _elementRef.nativeElement; // By calling this in the constructor we guarantee that the sibling sliders initial value by @@ -629,26 +628,26 @@ export class MatSlider extends _MatSliderMixinBase implements AfterViewInit, OnD } /** Gets the slider thumb input of the given thumb position. */ - _getInput(thumbPosition: Thumb): MatSliderThumb { + _getInput(thumbPosition: Thumb = Thumb.END): MatSliderThumb { return thumbPosition === Thumb.END ? this._inputs.last : this._inputs.first; } /** Gets the slider thumb HTML input element of the given thumb position. */ - _getInputElement(thumbPosition: Thumb): HTMLInputElement { + _getInputElement(thumbPosition: Thumb = Thumb.END): HTMLInputElement { return this._getInput(thumbPosition)._hostElement; } - private _getThumb(thumbPosition: Thumb): MatSliderVisualThumb { + _getThumb(thumbPosition: Thumb = Thumb.END): MatSliderVisualThumb { return thumbPosition === Thumb.END ? this._thumbs.last : this._thumbs.first; } /** Gets the slider thumb HTML element of the given thumb position. */ - _getThumbElement(thumbPosition: Thumb): HTMLElement { + _getThumbElement(thumbPosition: Thumb = Thumb.END): HTMLElement { return this._getThumb(thumbPosition)._getHostElement(); } /** Gets the slider knob HTML element of the given thumb position. */ - _getKnobElement(thumbPosition: Thumb): HTMLElement { + _getKnobElement(thumbPosition: Thumb = Thumb.END): HTMLElement { return this._getThumb(thumbPosition)._getKnob(); } diff --git a/src/material-experimental/mdc-theming/_all-theme.scss b/src/material-experimental/mdc-theming/_all-theme.scss index 843410f23827..83730bffb575 100644 --- a/src/material-experimental/mdc-theming/_all-theme.scss +++ b/src/material-experimental/mdc-theming/_all-theme.scss @@ -10,6 +10,7 @@ @use '../mdc-radio/radio-theme'; @use '../mdc-select/select-theme'; @use '../mdc-slide-toggle/slide-toggle-theme'; +@use '../mdc-slider/slider-theme'; @use '../mdc-snack-bar/snack-bar-theme'; @use '../mdc-tabs/tabs-theme'; @use '../mdc-table/table-theme'; @@ -42,6 +43,7 @@ @include radio-theme.theme($theme-or-color-config); @include select-theme.theme($theme-or-color-config); @include slide-toggle-theme.theme($theme-or-color-config); + @include slider-theme.theme($theme-or-color-config); @include snack-bar-theme.theme($theme-or-color-config); @include table-theme.theme($theme-or-color-config); @include form-field-theme.theme($theme-or-color-config); From d2d3ca5e1412fab4073b76dbd584f1b86b6f405e Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Tue, 2 Mar 2021 13:26:24 -0800 Subject: [PATCH 2/7] fix(material-experimental/mdc-slider): code review changes * use #waitForAsync instead of #fixture.whenStable --- .../mdc-slider/slider.spec.ts | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index 92ed2274cb91..922fbd34624d 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -8,7 +8,7 @@ import {dispatchMouseEvent, dispatchPointerEvent} from '@angular/cdk/testing/private'; import {Component, DebugElement, Type} from '@angular/core'; -import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; import {RippleRef, RippleState} from '@angular/material/core'; import {By} from '@angular/platform-browser'; import {Thumb} from '@material/slider'; @@ -35,17 +35,13 @@ describe('MDC-based MatSlider' , () => { let sliderInstance: MatSlider; let inputInstance: MatSliderThumb; - beforeEach(() => { + beforeEach(waitForAsync(() => { fixture = createComponent(StandardSlider); fixture.detectChanges(); sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); sliderInstance = sliderDebugElement.componentInstance; inputInstance = sliderInstance._getInput(Thumb.END); - }); - - beforeEach(done => { - fixture.whenStable().then(() => done()); - }); + })); it('should set the default values', () => { expect(inputInstance.value).toBe(0); @@ -87,18 +83,14 @@ describe('MDC-based MatSlider' , () => { let startInputInstance: MatSliderThumb; let endInputInstance: MatSliderThumb; - beforeEach(() => { + beforeEach(waitForAsync(() => { fixture = createComponent(StandardRangeSlider); fixture.detectChanges(); sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); sliderInstance = sliderDebugElement.componentInstance; startInputInstance = sliderInstance._getInput(Thumb.START); endInputInstance = sliderInstance._getInput(Thumb.END); - }); - - beforeEach(done => { - fixture.whenStable().then(() => done()); - }); + })); it('should set the default values', () => { expect(startInputInstance.value).toBe(0); @@ -160,7 +152,7 @@ describe('MDC-based MatSlider' , () => { let thumbX: number; let thumbY: number; - beforeEach(() => { + beforeEach(waitForAsync(() => { fixture = createComponent(StandardSlider); fixture.detectChanges(); const sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); @@ -168,11 +160,7 @@ describe('MDC-based MatSlider' , () => { inputInstance = sliderInstance._getInput(Thumb.END); thumbInstance = sliderInstance._getThumb(); thumbElement = thumbInstance._getHostElement(); - }); - - beforeEach(done => { - fixture.whenStable().then(() => done()); - }); + })); beforeEach(() => { const thumbDimensions = thumbElement.getBoundingClientRect(); From 9dfcd1348c0b5ab216a1a51f4e38d2ebad90f232 Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Fri, 5 Mar 2021 08:37:03 -0800 Subject: [PATCH 3/7] fix(material-experimental/mdc-slider): code review changes * remove default value from MatSlider getters * combine #beforeEach's in ripple state tests --- src/material-experimental/mdc-slider/slider.spec.ts | 7 ++----- src/material-experimental/mdc-slider/slider.ts | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index 922fbd34624d..3f948eda4d90 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -158,15 +158,12 @@ describe('MDC-based MatSlider' , () => { const sliderDebugElement = fixture.debugElement.query(By.directive(MatSlider)); const sliderInstance = sliderDebugElement.componentInstance; inputInstance = sliderInstance._getInput(Thumb.END); - thumbInstance = sliderInstance._getThumb(); + thumbInstance = sliderInstance._getThumb(Thumb.END); thumbElement = thumbInstance._getHostElement(); - })); - - beforeEach(() => { const thumbDimensions = thumbElement.getBoundingClientRect(); thumbX = thumbDimensions.left - (thumbDimensions.width / 2); thumbY = thumbDimensions.top - (thumbDimensions.height / 2); - }); + })); function isRippleVisible(rippleRef: RippleRef) { return rippleRef?.state === RippleState.FADING_IN diff --git a/src/material-experimental/mdc-slider/slider.ts b/src/material-experimental/mdc-slider/slider.ts index fd1c443a2aa5..c8e54c40bc78 100644 --- a/src/material-experimental/mdc-slider/slider.ts +++ b/src/material-experimental/mdc-slider/slider.ts @@ -628,26 +628,26 @@ export class MatSlider extends _MatSliderMixinBase implements AfterViewInit, OnD } /** Gets the slider thumb input of the given thumb position. */ - _getInput(thumbPosition: Thumb = Thumb.END): MatSliderThumb { + _getInput(thumbPosition: Thumb): MatSliderThumb { return thumbPosition === Thumb.END ? this._inputs.last : this._inputs.first; } /** Gets the slider thumb HTML input element of the given thumb position. */ - _getInputElement(thumbPosition: Thumb = Thumb.END): HTMLInputElement { + _getInputElement(thumbPosition: Thumb): HTMLInputElement { return this._getInput(thumbPosition)._hostElement; } - _getThumb(thumbPosition: Thumb = Thumb.END): MatSliderVisualThumb { + _getThumb(thumbPosition: Thumb): MatSliderVisualThumb { return thumbPosition === Thumb.END ? this._thumbs.last : this._thumbs.first; } /** Gets the slider thumb HTML element of the given thumb position. */ - _getThumbElement(thumbPosition: Thumb = Thumb.END): HTMLElement { + _getThumbElement(thumbPosition: Thumb): HTMLElement { return this._getThumb(thumbPosition)._getHostElement(); } /** Gets the slider knob HTML element of the given thumb position. */ - _getKnobElement(thumbPosition: Thumb = Thumb.END): HTMLElement { + _getKnobElement(thumbPosition: Thumb): HTMLElement { return this._getThumb(thumbPosition)._getKnob(); } From a55ceca96a60244f13d78a638ab6bd86ff408d09 Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Fri, 5 Mar 2021 08:43:19 -0800 Subject: [PATCH 4/7] fix(material-experimental/mdc-slider): code review changes * resolve injection error that only throws on ci --- src/material-experimental/mdc-slider/slider.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/material-experimental/mdc-slider/slider.ts b/src/material-experimental/mdc-slider/slider.ts index c8e54c40bc78..ea375ac2a433 100644 --- a/src/material-experimental/mdc-slider/slider.ts +++ b/src/material-experimental/mdc-slider/slider.ts @@ -23,6 +23,7 @@ import { Directive, ElementRef, EventEmitter, + forwardRef, Inject, Input, NgZone, @@ -110,7 +111,7 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy { constructor( private readonly _ngZone: NgZone, - private readonly _slider: MatSlider, + @Inject(forwardRef(() => MatSlider)) private readonly _slider: MatSlider, private readonly _elementRef: ElementRef) {} ngAfterViewInit() { @@ -331,7 +332,7 @@ export class MatSliderThumb implements AfterViewInit, ControlValueAccessor { constructor( @Inject(DOCUMENT) document: any, - private readonly _slider: MatSlider, + @Inject(forwardRef(() => MatSlider)) private readonly _slider: MatSlider, private readonly _elementRef: ElementRef) { this._document = document; this._hostElement = _elementRef.nativeElement; From cd2529614c0e33a58bb0fb686c03448efefe36a8 Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Fri, 5 Mar 2021 11:44:41 -0800 Subject: [PATCH 5/7] fix(material-experimental/mdc-slider): code review changes * make ripple refs private and check DOM directly in unit tests --- .../mdc-slider/slider.spec.ts | 87 +++++++++---------- .../mdc-slider/slider.ts | 6 +- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index 3f948eda4d90..d0596315fd27 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -8,8 +8,7 @@ import {dispatchMouseEvent, dispatchPointerEvent} from '@angular/cdk/testing/private'; import {Component, DebugElement, Type} from '@angular/core'; -import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; -import {RippleRef, RippleState} from '@angular/material/core'; +import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {Thumb} from '@material/slider'; import {MatSliderModule} from './module'; @@ -165,9 +164,9 @@ describe('MDC-based MatSlider' , () => { thumbY = thumbDimensions.top - (thumbDimensions.height / 2); })); - function isRippleVisible(rippleRef: RippleRef) { - return rippleRef?.state === RippleState.FADING_IN - || rippleRef?.state === RippleState.VISIBLE; + function isRippleVisible(selector: string) { + tick(500); + return !!document.querySelector(`.mat-mdc-slider-${selector}-ripple`); } function blur() { @@ -190,83 +189,83 @@ describe('MDC-based MatSlider' , () => { dispatchPointerEvent(thumbElement, 'pointerup', thumbX, thumbY); } - it('should show the hover ripple on mouseenter', () => { - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); + it('should show the hover ripple on mouseenter', fakeAsync(() => { + expect(isRippleVisible('hover')).toBe(false); mouseenter(); - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true); - }); + expect(isRippleVisible('hover')).toBe(true); + })); - it('should hide the hover ripple on mouseleave', () => { + it('should hide the hover ripple on mouseleave', fakeAsync(() => { mouseenter(); mouseleave(); - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); - }); + expect(isRippleVisible('hover')).toBe(false); + })); - it('should show the focus ripple on pointerdown', () => { - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); + it('should show the focus ripple on pointerdown', fakeAsync(() => { + expect(isRippleVisible('focus')).toBe(false); pointerdown(); - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); - }); + expect(isRippleVisible('focus')).toBe(true); + })); - it('should continue to show the focus ripple on pointerup', () => { + it('should continue to show the focus ripple on pointerup', fakeAsync(() => { pointerdown(); pointerup(); - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); - }); + expect(isRippleVisible('focus')).toBe(true); + })); - it('should hide the focus ripple on blur', () => { + it('should hide the focus ripple on blur', fakeAsync(() => { pointerdown(); pointerup(); blur(); - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); - }); + expect(isRippleVisible('focus')).toBe(false); + })); - it('should show the active ripple on pointerdown', () => { - expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false); + it('should show the active ripple on pointerdown', fakeAsync(() => { + expect(isRippleVisible('active')).toBe(false); pointerdown(); - expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(true); - }); + expect(isRippleVisible('active')).toBe(true); + })); - it('should hide the active ripple on pointerup', () => { + it('should hide the active ripple on pointerup', fakeAsync(() => { pointerdown(); pointerup(); - expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false); - }); + expect(isRippleVisible('active')).toBe(false); + })); // Edge cases. - it('should not show the hover ripple if the thumb is already focused', () => { + it('should not show the hover ripple if the thumb is already focused', fakeAsync(() => { pointerdown(); mouseenter(); - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); - }); + expect(isRippleVisible('hover')).toBe(false); + })); - it('should hide the hover ripple if the thumb is focused', () => { + it('should hide the hover ripple if the thumb is focused', fakeAsync(() => { mouseenter(); pointerdown(); - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false); - }); + expect(isRippleVisible('hover')).toBe(false); + })); - it('should not hide the focus ripple if the thumb is pressed', () => { + it('should not hide the focus ripple if the thumb is pressed', fakeAsync(() => { pointerdown(); blur(); - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true); - }); + expect(isRippleVisible('focus')).toBe(true); + })); - it('should not hide the hover ripple on blur if the cursor is thumb being hovered', () => { + it('should not hide the hover ripple on blur if the thumb is hovered', fakeAsync(() => { mouseenter(); pointerdown(); pointerup(); blur(); - expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true); - }); + expect(isRippleVisible('hover')).toBe(true); + })); - it('should hide the focus ripple on drag end if the thumb already lost focus', () => { + it('should hide the focus ripple on drag end if the thumb already lost focus', fakeAsync(() => { pointerdown(); blur(); pointerup(); - expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false); - }); + expect(isRippleVisible('focus')).toBe(false); + })); }); }); diff --git a/src/material-experimental/mdc-slider/slider.ts b/src/material-experimental/mdc-slider/slider.ts index ea375ac2a433..90753fbaf329 100644 --- a/src/material-experimental/mdc-slider/slider.ts +++ b/src/material-experimental/mdc-slider/slider.ts @@ -95,13 +95,13 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy { private _sliderInput: MatSliderThumb; /** The RippleRef for the slider thumbs hover state. */ - _hoverRippleRef: RippleRef; + private _hoverRippleRef: RippleRef; /** The RippleRef for the slider thumbs focus state. */ - _focusRippleRef: RippleRef; + private _focusRippleRef: RippleRef; /** The RippleRef for the slider thumbs active state. */ - _activeRippleRef: RippleRef; + private _activeRippleRef: RippleRef; /** Whether the slider thumb is currently being pressed. */ private _isActive: boolean = false; From 10035f1275378009d3b7448bb60e5351597750e1 Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Mon, 8 Mar 2021 09:59:07 -0800 Subject: [PATCH 6/7] fix(material-experimental/mdc-slider): disable mat ripple * Disable the mat ripple on the slider thumbs to prevent the automatic launch that happens on click/touch. The problem is easily reproduced if you undo this change and test it out on a mobile device. --- src/material-experimental/mdc-slider/slider-thumb.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/material-experimental/mdc-slider/slider-thumb.html b/src/material-experimental/mdc-slider/slider-thumb.html index da92e3e64b9c..595c373fe1f7 100644 --- a/src/material-experimental/mdc-slider/slider-thumb.html +++ b/src/material-experimental/mdc-slider/slider-thumb.html @@ -4,4 +4,4 @@
-
+
From 17d55b2f3dfc0c0f8c67613490448f545e44ef0e Mon Sep 17 00:00:00 2001 From: wagnermaciel Date: Mon, 8 Mar 2021 10:01:07 -0800 Subject: [PATCH 7/7] fix(material-experimental/mdc-slider): fix ios unit test bug * use touch events instead of pointer events when testing on ios. pointerdown, pointerup, and pointermove are not supported on ios. --- .../testbed/fake-events/dispatch-events.ts | 5 +- .../testbed/fake-events/event-objects.ts | 4 +- .../mdc-slider/slider.spec.ts | 94 ++++++++++++++----- 3 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/cdk/testing/testbed/fake-events/dispatch-events.ts b/src/cdk/testing/testbed/fake-events/dispatch-events.ts index ee4759ac0679..4063386d1215 100644 --- a/src/cdk/testing/testbed/fake-events/dispatch-events.ts +++ b/src/cdk/testing/testbed/fake-events/dispatch-events.ts @@ -65,6 +65,7 @@ export function dispatchPointerEvent(node: Node, type: string, clientX = 0, clie * Shorthand to dispatch a touch event on the specified coordinates. * @docs-private */ -export function dispatchTouchEvent(node: Node, type: string, x = 0, y = 0) { - return dispatchEvent(node, createTouchEvent(type, x, y)); +export function dispatchTouchEvent(node: Node, type: string, pageX = 0, pageY = 0, clientX = 0, + clientY = 0) { + return dispatchEvent(node, createTouchEvent(type, pageX, pageY, clientX, clientY)); } diff --git a/src/cdk/testing/testbed/fake-events/event-objects.ts b/src/cdk/testing/testbed/fake-events/event-objects.ts index f211df2920f6..a22cb64be43c 100644 --- a/src/cdk/testing/testbed/fake-events/event-objects.ts +++ b/src/cdk/testing/testbed/fake-events/event-objects.ts @@ -79,11 +79,11 @@ export function createPointerEvent(type: string, clientX = 0, clientY = 0, * Creates a browser TouchEvent with the specified pointer coordinates. * @docs-private */ -export function createTouchEvent(type: string, pageX = 0, pageY = 0) { +export function createTouchEvent(type: string, pageX = 0, pageY = 0, clientX = 0, clientY = 0) { // In favor of creating events that work for most of the browsers, the event is created // as a basic UI Event. The necessary details for the event will be set manually. const event = document.createEvent('UIEvent'); - const touchDetails = {pageX, pageY}; + const touchDetails = {pageX, pageY, clientX, clientY}; // TS3.6 removes the initUIEvent method and suggests porting to "new UIEvent()". (event as any).initUIEvent(type, true, true, window, 0); diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index d0596315fd27..8ae842f776e4 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -6,7 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {dispatchMouseEvent, dispatchPointerEvent} from '@angular/cdk/testing/private'; +import {Platform} from '@angular/cdk/platform'; +import { + dispatchMouseEvent, + dispatchPointerEvent, + dispatchTouchEvent, +} from '@angular/cdk/testing/private'; import {Component, DebugElement, Type} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -15,7 +20,10 @@ import {MatSliderModule} from './module'; import {MatSlider, MatSliderThumb, MatSliderVisualThumb} from './slider'; describe('MDC-based MatSlider' , () => { + let platform: Platform; + beforeAll(() => { + platform = TestBed.inject(Platform); // Mock #setPointerCapture as it throws errors on pointerdown without a real pointerId. spyOn(Element.prototype, 'setPointerCapture'); }); @@ -49,28 +57,28 @@ describe('MDC-based MatSlider' , () => { }); it('should update the value on mousedown', () => { - setValueByClick(sliderInstance, 19); + setValueByClick(sliderInstance, 19, platform.IOS); expect(inputInstance.value).toBe(19); }); it('should update the value on a slide', () => { - slideToValue(sliderInstance, 77); + slideToValue(sliderInstance, 77, Thumb.END, platform.IOS); expect(inputInstance.value).toBe(77); }); it('should set the value as min when sliding before the track', () => { - slideToValue(sliderInstance, -1); + slideToValue(sliderInstance, -1, Thumb.END, platform.IOS); expect(inputInstance.value).toBe(0); }); it('should set the value as max when sliding past the track', () => { - slideToValue(sliderInstance, 101); + slideToValue(sliderInstance, 101, Thumb.END, platform.IOS); expect(inputInstance.value).toBe(100); }); it('should focus the slider input when clicking on the slider', () => { expect(document.activeElement).not.toBe(inputInstance._hostElement); - setValueByClick(sliderInstance, 0); + setValueByClick(sliderInstance, 0, platform.IOS); expect(document.activeElement).toBe(inputInstance._hostElement); }); }); @@ -99,46 +107,46 @@ describe('MDC-based MatSlider' , () => { }); it('should update the start value on a slide', () => { - slideToValue(sliderInstance, 19, Thumb.START); + slideToValue(sliderInstance, 19, Thumb.START, platform.IOS); expect(startInputInstance.value).toBe(19); }); it('should update the end value on a slide', () => { - slideToValue(sliderInstance, 27, Thumb.END); + slideToValue(sliderInstance, 27, Thumb.END, platform.IOS); expect(endInputInstance.value).toBe(27); }); it('should update the start value on mousedown behind the start thumb', () => { sliderInstance._setValue(19, Thumb.START); - setValueByClick(sliderInstance, 12); + setValueByClick(sliderInstance, 12, platform.IOS); expect(startInputInstance.value).toBe(12); }); it('should update the end value on mousedown in front of the end thumb', () => { sliderInstance._setValue(27, Thumb.END); - setValueByClick(sliderInstance, 55); + setValueByClick(sliderInstance, 55, platform.IOS); expect(endInputInstance.value).toBe(55); }); it('should set the start value as min when sliding before the track', () => { - slideToValue(sliderInstance, -1, Thumb.START); + slideToValue(sliderInstance, -1, Thumb.START, platform.IOS); expect(startInputInstance.value).toBe(0); }); it('should set the end value as max when sliding past the track', () => { - slideToValue(sliderInstance, 101, Thumb.START); + slideToValue(sliderInstance, 101, Thumb.START, platform.IOS); expect(startInputInstance.value).toBe(100); }); it('should not let the start thumb slide past the end thumb', () => { sliderInstance._setValue(50, Thumb.END); - slideToValue(sliderInstance, 75, Thumb.START); + slideToValue(sliderInstance, 75, Thumb.START, platform.IOS); expect(startInputInstance.value).toBe(50); }); it('should not let the end thumb slide before the start thumb', () => { sliderInstance._setValue(50, Thumb.START); - slideToValue(sliderInstance, 25, Thumb.END); + slideToValue(sliderInstance, 25, Thumb.END, platform.IOS); expect(startInputInstance.value).toBe(50); }); }); @@ -182,11 +190,15 @@ describe('MDC-based MatSlider' , () => { } function pointerdown() { - dispatchPointerEvent(thumbElement, 'pointerdown', thumbX, thumbY); + dispatchPointerOrTouchEvent( + thumbElement, PointerEventType.POINTER_DOWN, thumbX, thumbY, platform.IOS + ); } function pointerup() { - dispatchPointerEvent(thumbElement, 'pointerup', thumbX, thumbY); + dispatchPointerOrTouchEvent( + thumbElement, PointerEventType.POINTER_UP, thumbX, thumbY, platform.IOS + ); } it('should show the hover ripple on mouseenter', fakeAsync(() => { @@ -289,8 +301,22 @@ class StandardSlider {} }) class StandardRangeSlider {} +/** The pointer event types used by the MDC Slider. */ +const enum PointerEventType { + POINTER_DOWN = 'pointerdown', + POINTER_UP = 'pointerup', + POINTER_MOVE = 'pointermove', +} + +/** The touch event types used by the MDC Slider. */ +const enum TouchEventType { + TOUCH_START = 'touchstart', + TOUCH_END = 'touchend', + TOUCH_MOVE = 'touchmove', +} + /** Clicks on the MatSlider at the coordinates corresponding to the given value. */ -function setValueByClick(slider: MatSlider, value: number) { +function setValueByClick(slider: MatSlider, value: number, isIOS: boolean) { const {min, max} = slider; const percent = (value - min) / (max - min); @@ -299,12 +325,12 @@ function setValueByClick(slider: MatSlider, value: number) { const x = left + (width * percent); const y = top + (height / 2); - dispatchPointerEvent(sliderElement, 'pointerdown', x, y); - dispatchPointerEvent(sliderElement, 'pointerup', x, y); + dispatchPointerOrTouchEvent(sliderElement, PointerEventType.POINTER_DOWN, x, y, isIOS); + dispatchPointerOrTouchEvent(sliderElement, PointerEventType.POINTER_UP, x, y, isIOS); } /** Slides the MatSlider's thumb to the given value. */ -function slideToValue(slider: MatSlider, value: number, thumbPosition: Thumb = Thumb.END) { +function slideToValue(slider: MatSlider, value: number, thumbPosition: Thumb, isIOS: boolean) { const {min, max} = slider; const percent = (value - min) / (max - min); @@ -320,7 +346,29 @@ function slideToValue(slider: MatSlider, value: number, thumbPosition: Thumb = T const endX = sliderDimensions.left + (sliderDimensions.width * percent); const endY = sliderDimensions.top + (sliderDimensions.height / 2); - dispatchPointerEvent(sliderElement, 'pointerdown', startX, startY); - dispatchPointerEvent(sliderElement, 'pointermove', endX, endY); - dispatchPointerEvent(sliderElement, 'pointerup', endX, endY); + dispatchPointerOrTouchEvent(sliderElement, PointerEventType.POINTER_DOWN, startX, startY, isIOS); + dispatchPointerOrTouchEvent(sliderElement, PointerEventType.POINTER_MOVE, endX, endY, isIOS); + dispatchPointerOrTouchEvent(sliderElement, PointerEventType.POINTER_UP, endX, endY, isIOS); +} + +/** Dispatch a pointerdown or pointerup event if supported, otherwise dispatch the touch event. */ +function dispatchPointerOrTouchEvent( + node: Node, type: PointerEventType, x: number, y: number, isIOS: boolean) { + if (isIOS) { + dispatchTouchEvent(node, pointerEventTypeToTouchEventType(type), x, y, x, y); + } else { + dispatchPointerEvent(node, type, x, y); + } +} + +/** Returns the touch event equivalent of the given pointer event. */ +function pointerEventTypeToTouchEventType(pointerEventType: PointerEventType) { + switch (pointerEventType) { + case PointerEventType.POINTER_DOWN: + return TouchEventType.TOUCH_START; + case PointerEventType.POINTER_UP: + return TouchEventType.TOUCH_END; + case PointerEventType.POINTER_MOVE: + return TouchEventType.TOUCH_MOVE; + } }