From d8643ed4fdcf1350257f91fd56d416b772fb774e Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 19 Sep 2016 20:24:39 +0200 Subject: [PATCH 1/4] fix(slide-toggle): disabled theme and dragging * It seems like as per the new theming feature, view encapsulation turned off and now the checked theming overwrites the disabled theme (too high specificity) * If a slide-toggle is disabled, users are still able to drag the thumb (which is invalid) * Fix invalid `user-select` property, and now dragging works without clamps. --- src/lib/slide-toggle/_slide-toggle-theme.scss | 4 +++- src/lib/slide-toggle/slide-toggle.scss | 7 +++++- src/lib/slide-toggle/slide-toggle.ts | 22 ++++++++++++------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/lib/slide-toggle/_slide-toggle-theme.scss b/src/lib/slide-toggle/_slide-toggle-theme.scss index 338dfb691164..c3f4181a42bd 100644 --- a/src/lib/slide-toggle/_slide-toggle-theme.scss +++ b/src/lib/slide-toggle/_slide-toggle-theme.scss @@ -3,7 +3,9 @@ @mixin _md-slide-toggle-checked($palette) { - &.md-checked { + // Do not apply the checked colors if the toggle is disabled, because the specificity would be to high for + // the disabled styles. + &.md-checked:not(.md-disabled) { .md-slide-toggle-thumb { background-color: md-color($palette); } diff --git a/src/lib/slide-toggle/slide-toggle.scss b/src/lib/slide-toggle/slide-toggle.scss index 02ac7f75987f..a1068e60e295 100644 --- a/src/lib/slide-toggle/slide-toggle.scss +++ b/src/lib/slide-toggle/slide-toggle.scss @@ -24,6 +24,12 @@ $md-slide-toggle-margin: 16px !default; line-height: $md-slide-toggle-height; white-space: nowrap; + + // Disable user selection to ensure that dragging is smooth without grabbing some elements + // accidentally. Manually prefixing here, because the un-prefixed property is not supported yet. + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; user-select: none; outline: none; @@ -68,7 +74,6 @@ $md-slide-toggle-margin: 16px !default; height: $md-slide-toggle-height; position: relative; - user-select: none; margin-right: 8px; } diff --git a/src/lib/slide-toggle/slide-toggle.ts b/src/lib/slide-toggle/slide-toggle.ts index 1127e9258284..dbe797589db4 100644 --- a/src/lib/slide-toggle/slide-toggle.ts +++ b/src/lib/slide-toggle/slide-toggle.ts @@ -215,16 +215,24 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor { /** TODO: internal */ _onDragStart() { - this._slideRenderer.startThumbDrag(this.checked); + if (!this.disabled) { + this._slideRenderer.startThumbDrag(this.checked); + } } /** TODO: internal */ _onDrag(event: HammerInput) { - this._slideRenderer.updateThumbPosition(event.deltaX); + if (this._slideRenderer.isDragging()) { + this._slideRenderer.updateThumbPosition(event.deltaX); + } } /** TODO: internal */ _onDragEnd() { + if (!this._slideRenderer.isDragging()) { + return; + } + // Notice that we have to stop outside of the current event handler, // because otherwise the click event will be fired and will reset the new checked variable. setTimeout(() => { @@ -258,7 +266,7 @@ class SlideToggleRenderer { /** Initializes the drag of the slide-toggle. */ startThumbDrag(checked: boolean) { - if (!this._thumbBarWidth) { + if (!this.isDragging()) { this._thumbBarWidth = this._thumbBarEl.clientWidth - this._thumbEl.clientWidth; this._checked = checked; this._thumbEl.classList.add('md-dragging'); @@ -267,7 +275,7 @@ class SlideToggleRenderer { /** Stops the current drag and returns the new checked value. */ stopThumbDrag(): boolean { - if (this._thumbBarWidth) { + if (this.isDragging()) { this._thumbBarWidth = null; this._thumbEl.classList.remove('md-dragging'); @@ -279,10 +287,8 @@ class SlideToggleRenderer { /** Updates the thumb containers position from the specified distance. */ updateThumbPosition(distance: number) { - if (this._thumbBarWidth) { - this._percentage = this._getThumbPercentage(distance); - applyCssTransform(this._thumbEl, `translate3d(${this._percentage}%, 0, 0)`); - } + this._percentage = this._getThumbPercentage(distance); + applyCssTransform(this._thumbEl, `translate3d(${this._percentage}%, 0, 0)`); } /** Retrieves the percentage of thumb from the moved distance. */ From 4573474adfa537b6d82568df244522fb7e2e01e3 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 2 Oct 2016 17:17:43 +0200 Subject: [PATCH 2/4] =?UTF-8?q?=EF=BB=BFAdded=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/slide-toggle/slide-toggle.spec.ts | 108 +++++++++++++++++++++- src/lib/slider/test-gesture-config.ts | 2 +- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/lib/slide-toggle/slide-toggle.spec.ts b/src/lib/slide-toggle/slide-toggle.spec.ts index a810d2185110..ed34bccf9758 100644 --- a/src/lib/slide-toggle/slide-toggle.spec.ts +++ b/src/lib/slide-toggle/slide-toggle.spec.ts @@ -1,15 +1,21 @@ -import {async, ComponentFixture, TestBed} from '@angular/core/testing'; -import {By} from '@angular/platform-browser'; +import {async, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing'; +import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser'; import {Component} from '@angular/core'; import {MdSlideToggle, MdSlideToggleChange, MdSlideToggleModule} from './slide-toggle'; import {FormsModule, NgControl} from '@angular/forms'; +import {TestGestureConfig} from '../slider/test-gesture-config'; describe('MdSlideToggle', () => { + let gestureConfig: TestGestureConfig; + beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdSlideToggleModule.forRoot(), FormsModule], declarations: [SlideToggleTestApp, SlideToggleFormsTestApp], + providers: [ + {provide: HAMMER_GESTURE_CONFIG, useFactory: () => gestureConfig = new TestGestureConfig()} + ] }); TestBed.compileComponents(); @@ -392,6 +398,103 @@ describe('MdSlideToggle', () => { }); + describe('with dragging', () => { + + let fixture: ComponentFixture; + + let testComponent: SlideToggleTestApp; + let slideToggle: MdSlideToggle; + let slideToggleElement: HTMLElement; + let slideToggleControl: NgControl; + let slideThumbContainer: HTMLElement; + + // This initialization is async() because it needs to wait for ngModel to set the initial value. + beforeEach(async(() => { + fixture = TestBed.createComponent(SlideToggleTestApp); + + testComponent = fixture.debugElement.componentInstance; + + // Enable jasmine spies on event functions, which may trigger at initialization + // of the slide-toggle component. + spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough(); + spyOn(fixture.debugElement.componentInstance, 'onSlideClick').and.callThrough(); + + // Initialize the slide-toggle component, by triggering the first change detection cycle. + fixture.detectChanges(); + + let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle')); + let thumbContainerDebug = slideToggleDebug.query(By.css('.md-slide-toggle-thumb-container')); + + slideToggle = slideToggleDebug.componentInstance; + slideToggleElement = slideToggleDebug.nativeElement; + slideToggleControl = slideToggleDebug.injector.get(NgControl); + slideThumbContainer = thumbContainerDebug.nativeElement; + })); + + it('should drag from start to end', fakeAsync(() => { + expect(slideToggle.checked).toBe(false); + + gestureConfig.emitEventForElement('slidestart', slideThumbContainer); + + expect(slideThumbContainer.classList).toContain('md-dragging'); + + gestureConfig.emitEventForElement('slide', slideThumbContainer, { + deltaX: 200 // Use a random number which will be clamped. + }); + + gestureConfig.emitEventForElement('slideend', slideThumbContainer); + + // Flush the timeout for the slide ending. + tick(); + + expect(slideToggle.checked).toBe(true); + expect(slideThumbContainer.classList).not.toContain('md-dragging'); + })); + + it('should drag from end to start', fakeAsync(() => { + slideToggle.checked = true; + + gestureConfig.emitEventForElement('slidestart', slideThumbContainer); + + expect(slideThumbContainer.classList).toContain('md-dragging'); + + gestureConfig.emitEventForElement('slide', slideThumbContainer, { + deltaX: -200 // Use a random negative number which will be clamped. + }); + + gestureConfig.emitEventForElement('slideend', slideThumbContainer); + + // Flush the timeout for the slide ending. + tick(); + + expect(slideToggle.checked).toBe(false); + expect(slideThumbContainer.classList).not.toContain('md-dragging'); + })); + + it('should not drag when disbaled', fakeAsync(() => { + slideToggle.disabled = true; + + expect(slideToggle.checked).toBe(false); + + gestureConfig.emitEventForElement('slidestart', slideThumbContainer); + + expect(slideThumbContainer.classList).not.toContain('md-dragging'); + + gestureConfig.emitEventForElement('slide', slideThumbContainer, { + deltaX: 200 // Use a random number which will be clamped. + }); + + gestureConfig.emitEventForElement('slideend', slideThumbContainer); + + // Flush the timeout for the slide ending. + tick(); + + expect(slideToggle.checked).toBe(false); + expect(slideThumbContainer.classList).not.toContain('md-dragging'); + })); + + }); + }); /** @@ -405,6 +508,7 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void element.dispatchEvent(event); } + @Component({ selector: 'slide-toggle-test-app', template: ` diff --git a/src/lib/slider/test-gesture-config.ts b/src/lib/slider/test-gesture-config.ts index 78040bb665f1..d9e83d28ad07 100644 --- a/src/lib/slider/test-gesture-config.ts +++ b/src/lib/slider/test-gesture-config.ts @@ -32,7 +32,7 @@ export class TestGestureConfig extends MdGestureConfig { * The Angular event plugin for Hammer creates a new HammerManager instance for each listener, * so we need to apply our event on all instances to hit the correct listener. */ - emitEventForElement(eventType: string, element: HTMLElement, eventData: Object) { + emitEventForElement(eventType: string, element: HTMLElement, eventData = {}) { let instances = this.hammerInstances.get(element); instances.forEach(instance => instance.emit(eventType, eventData)); } From 927f2b1416b28c27a7c8841215a492137a77c381 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 2 Oct 2016 17:53:42 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=EF=BB=BFRemove=20whitespace=20line?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/slide-toggle/slide-toggle.spec.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/lib/slide-toggle/slide-toggle.spec.ts b/src/lib/slide-toggle/slide-toggle.spec.ts index ed34bccf9758..75c446678a5e 100644 --- a/src/lib/slide-toggle/slide-toggle.spec.ts +++ b/src/lib/slide-toggle/slide-toggle.spec.ts @@ -408,18 +408,11 @@ describe('MdSlideToggle', () => { let slideToggleControl: NgControl; let slideThumbContainer: HTMLElement; - // This initialization is async() because it needs to wait for ngModel to set the initial value. beforeEach(async(() => { fixture = TestBed.createComponent(SlideToggleTestApp); testComponent = fixture.debugElement.componentInstance; - // Enable jasmine spies on event functions, which may trigger at initialization - // of the slide-toggle component. - spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough(); - spyOn(fixture.debugElement.componentInstance, 'onSlideClick').and.callThrough(); - - // Initialize the slide-toggle component, by triggering the first change detection cycle. fixture.detectChanges(); let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle')); @@ -508,7 +501,6 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void element.dispatchEvent(event); } - @Component({ selector: 'slide-toggle-test-app', template: ` From 2ee564046f18e1471d87c0912bc8284b903a3cc1 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 5 Oct 2016 20:20:25 +0200 Subject: [PATCH 4/4] =?UTF-8?q?=EF=BB=BFFix=20comment=20wording.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/slide-toggle/slide-toggle.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/slide-toggle/slide-toggle.spec.ts b/src/lib/slide-toggle/slide-toggle.spec.ts index 75c446678a5e..cc0080b02795 100644 --- a/src/lib/slide-toggle/slide-toggle.spec.ts +++ b/src/lib/slide-toggle/slide-toggle.spec.ts @@ -432,7 +432,7 @@ describe('MdSlideToggle', () => { expect(slideThumbContainer.classList).toContain('md-dragging'); gestureConfig.emitEventForElement('slide', slideThumbContainer, { - deltaX: 200 // Use a random number which will be clamped. + deltaX: 200 // Arbitrary, large delta that will be clamped to the end of the slide-toggle. }); gestureConfig.emitEventForElement('slideend', slideThumbContainer); @@ -452,7 +452,7 @@ describe('MdSlideToggle', () => { expect(slideThumbContainer.classList).toContain('md-dragging'); gestureConfig.emitEventForElement('slide', slideThumbContainer, { - deltaX: -200 // Use a random negative number which will be clamped. + deltaX: -200 // Arbitrary, large delta that will be clamped to the end of the slide-toggle. }); gestureConfig.emitEventForElement('slideend', slideThumbContainer); @@ -474,7 +474,7 @@ describe('MdSlideToggle', () => { expect(slideThumbContainer.classList).not.toContain('md-dragging'); gestureConfig.emitEventForElement('slide', slideThumbContainer, { - deltaX: 200 // Use a random number which will be clamped. + deltaX: 200 // Arbitrary, large delta that will be clamped to the end of the slide-toggle. }); gestureConfig.emitEventForElement('slideend', slideThumbContainer);