From 81af6443f143791bbb01ace855cc3a5317b93a55 Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Tue, 29 Aug 2017 11:03:30 -0700 Subject: [PATCH 1/6] Fix focus change for updated slider --- src/lib/slider/slider.spec.ts | 11 +++++++++++ src/lib/slider/slider.ts | 1 + 2 files changed, 12 insertions(+) diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index 3bdeff346d13..f6d650cb7156 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -141,6 +141,17 @@ describe('MdSlider without forms', () => { expect(sliderNativeElement.classList).not.toContain('mat-slider-sliding'); }); + it('should remove focus after the slider is updated', () => { + spyOn(sliderNativeElement, 'blur'); + + expect(sliderNativeElement.blur).not.toHaveBeenCalled(); + + dispatchClickEventSequence(sliderNativeElement, 0.39); + fixture.detectChanges(); + + expect(sliderNativeElement.blur).toHaveBeenCalled(); + }); + it('should have thumb gap when at min value', () => { expect(trackFillElement.style.transform).toContain('translateX(-7px)'); }); diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 701c08b3396c..6c2eee68c297 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -445,6 +445,7 @@ export class MdSlider extends _MdSliderMixinBase this._emitInputEvent(); this._emitChangeEvent(); } + this._elementRef.nativeElement.blur(); } _onSlide(event: HammerInput) { From 677907646c3e97e00360551ccf1599e37ab21e8a Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Wed, 30 Aug 2017 14:35:23 -0700 Subject: [PATCH 2/6] Fix blur event and directionality bug --- src/lib/slider/slider.spec.ts | 30 +++++++++++++++++++++++------- src/lib/slider/slider.ts | 9 ++++++++- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index f6d650cb7156..1c87627f6881 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -16,7 +16,7 @@ import { UP_ARROW, BACKSPACE } from '../core/keyboard/keycodes'; -import {dispatchKeyboardEvent, dispatchMouseEvent} from '@angular/cdk/testing'; +import {dispatchFakeEvent, dispatchKeyboardEvent, dispatchMouseEvent} from '@angular/cdk/testing'; describe('MdSlider without forms', () => { let gestureConfig: TestGestureConfig; @@ -141,15 +141,13 @@ describe('MdSlider without forms', () => { expect(sliderNativeElement.classList).not.toContain('mat-slider-sliding'); }); - it('should remove focus after the slider is updated', () => { - spyOn(sliderNativeElement, 'blur'); + it('should reset active state upon blur', () => { + sliderInstance._isActive = true; - expect(sliderNativeElement.blur).not.toHaveBeenCalled(); - - dispatchClickEventSequence(sliderNativeElement, 0.39); + dispatchFakeEvent(sliderNativeElement, 'blur'); fixture.detectChanges(); - expect(sliderNativeElement.blur).toHaveBeenCalled(); + expect(sliderInstance._isActive).toBe(false); }); it('should have thumb gap when at min value', () => { @@ -969,6 +967,24 @@ describe('MdSlider without forms', () => { expect(sliderInstance.value).toBe(30); }); + it('should re-render slider with updated style upon directionality change', () => { + testComponent.dir = 'rtl'; + fixture.detectChanges(); + + let initialTrackFillStyles = sliderInstance._trackFillStyles; + let initialTicksContainerStyles = sliderInstance._ticksContainerStyles; + let initialTicksStyles = sliderInstance._ticksStyles; + let initialThumbContainerStyles = sliderInstance._thumbContainerStyles; + + testComponent.dir = 'ltr'; + fixture.detectChanges(); + + expect(initialTrackFillStyles).not.toEqual(sliderInstance._trackFillStyles); + expect(initialTicksContainerStyles).not.toEqual(sliderInstance._ticksContainerStyles); + expect(initialTicksStyles).not.toEqual(sliderInstance._ticksStyles); + expect(initialThumbContainerStyles).not.toEqual(sliderInstance._thumbContainerStyles); + }); + it('should increment inverted slider by 1 on right arrow pressed', () => { testComponent.invert = true; fixture.detectChanges(); diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 6c2eee68c297..60ea4058ec1a 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -413,10 +413,16 @@ export class MdSlider extends _MdSliderMixinBase this._focusOriginMonitor .monitor(this._elementRef.nativeElement, renderer, true) .subscribe((origin: FocusOrigin) => this._isActive = !!origin && origin !== 'keyboard'); + if (_dir) { + _dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); + } } ngOnDestroy() { this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement); + if (this._dir) { + this._dir.change.unsubscribe(); + } } _onMouseenter() { @@ -445,7 +451,6 @@ export class MdSlider extends _MdSliderMixinBase this._emitInputEvent(); this._emitChangeEvent(); } - this._elementRef.nativeElement.blur(); } _onSlide(event: HammerInput) { @@ -507,6 +512,8 @@ export class MdSlider extends _MdSliderMixinBase _onBlur() { this.onTouched(); + this._isActive = false; + this._changeDetectorRef.markForCheck(); } _onKeydown(event: KeyboardEvent) { From 4118f4e5f697ce2b8492e2be06b982fe3b27d5e7 Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Wed, 30 Aug 2017 15:50:51 -0700 Subject: [PATCH 3/6] Modify change detection --- src/lib/slider/slider.spec.ts | 13 +++++++++++++ src/lib/slider/slider.ts | 7 ++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index 1c87627f6881..676929861740 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -150,6 +150,19 @@ describe('MdSlider without forms', () => { expect(sliderInstance._isActive).toBe(false); }); + it('should reset thumb gap when blurred on min value', () => { + sliderInstance._isActive = true; + sliderInstance.value = 0; + fixture.detectChanges(); + + expect(sliderInstance._thumbGap).toBe(10); + + dispatchFakeEvent(sliderNativeElement, 'blur'); + fixture.detectChanges(); + + expect(sliderInstance._thumbGap).toBe(7); + }); + it('should have thumb gap when at min value', () => { expect(trackFillElement.style.transform).toContain('translateX(-7px)'); }); diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 60ea4058ec1a..4ef5e4b00474 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -412,7 +412,10 @@ export class MdSlider extends _MdSliderMixinBase super(renderer, elementRef); this._focusOriginMonitor .monitor(this._elementRef.nativeElement, renderer, true) - .subscribe((origin: FocusOrigin) => this._isActive = !!origin && origin !== 'keyboard'); + .subscribe((origin: FocusOrigin) => { + this._isActive = !!origin && origin !== 'keyboard'; + this._changeDetectorRef.detectChanges(); + }); if (_dir) { _dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); } @@ -512,8 +515,6 @@ export class MdSlider extends _MdSliderMixinBase _onBlur() { this.onTouched(); - this._isActive = false; - this._changeDetectorRef.markForCheck(); } _onKeydown(event: KeyboardEvent) { From e446bef9ff436f7e7de5e6a328e26e2eb0968132 Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Wed, 30 Aug 2017 16:22:48 -0700 Subject: [PATCH 4/6] Fix prerender test issue --- src/lib/slider/slider.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 4ef5e4b00474..2608f3c6e00c 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -388,6 +388,9 @@ export class MdSlider extends _MdSliderMixinBase /** The value of the slider when the slide start event fires. */ private _valueOnSlideStart: number | null; + /** Whether the component has been initialized. */ + private _initialized = false; + /** Reference to the inner slider wrapper element. */ @ViewChild('sliderWrapper') private _sliderWrapper: ElementRef; @@ -414,10 +417,16 @@ export class MdSlider extends _MdSliderMixinBase .monitor(this._elementRef.nativeElement, renderer, true) .subscribe((origin: FocusOrigin) => { this._isActive = !!origin && origin !== 'keyboard'; - this._changeDetectorRef.detectChanges(); + if (this._initialized) { + this._changeDetectorRef.detectChanges(); + } }); - if (_dir) { - _dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); + } + + ngOnInit() { + this._initialized = true; + if (this._dir) { + this._dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); } } From ccd798bcf0f0208665a5f4b9233125c074a42d9c Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Thu, 31 Aug 2017 10:05:47 -0700 Subject: [PATCH 5/6] Move subscriptions into ngOnInit --- src/lib/slider/slider.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 2608f3c6e00c..c054fb6d6c55 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -388,9 +388,6 @@ export class MdSlider extends _MdSliderMixinBase /** The value of the slider when the slide start event fires. */ private _valueOnSlideStart: number | null; - /** Whether the component has been initialized. */ - private _initialized = false; - /** Reference to the inner slider wrapper element. */ @ViewChild('sliderWrapper') private _sliderWrapper: ElementRef; @@ -413,18 +410,15 @@ export class MdSlider extends _MdSliderMixinBase private _changeDetectorRef: ChangeDetectorRef, @Optional() private _dir: Directionality) { super(renderer, elementRef); + } + + ngOnInit() { this._focusOriginMonitor - .monitor(this._elementRef.nativeElement, renderer, true) + .monitor(this._elementRef.nativeElement, this._renderer, true) .subscribe((origin: FocusOrigin) => { this._isActive = !!origin && origin !== 'keyboard'; - if (this._initialized) { - this._changeDetectorRef.detectChanges(); - } + this._changeDetectorRef.detectChanges(); }); - } - - ngOnInit() { - this._initialized = true; if (this._dir) { this._dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); } From d44e19ed31c95f0aff66ae3caf78412226ebf152 Mon Sep 17 00:00:00 2001 From: Ji Won Shin Date: Thu, 31 Aug 2017 10:16:28 -0700 Subject: [PATCH 6/6] address comments --- src/lib/slider/slider.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index c054fb6d6c55..e794e1f4575a 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -19,7 +19,7 @@ import { Output, Renderer2, ViewEncapsulation, - ViewChild, + ViewChild, OnInit, } from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion'; @@ -125,7 +125,7 @@ export const _MdSliderMixinBase = mixinColor(mixinDisabled(MdSliderBase), 'accen changeDetection: ChangeDetectionStrategy.OnPush, }) export class MdSlider extends _MdSliderMixinBase - implements ControlValueAccessor, OnDestroy, CanDisable, CanColor { + implements ControlValueAccessor, OnDestroy, CanDisable, CanColor, OnInit { /** Whether the slider is inverted. */ @Input() get invert() { return this._invert; } @@ -416,9 +416,9 @@ export class MdSlider extends _MdSliderMixinBase this._focusOriginMonitor .monitor(this._elementRef.nativeElement, this._renderer, true) .subscribe((origin: FocusOrigin) => { - this._isActive = !!origin && origin !== 'keyboard'; - this._changeDetectorRef.detectChanges(); - }); + this._isActive = !!origin && origin !== 'keyboard'; + this._changeDetectorRef.detectChanges(); + }); if (this._dir) { this._dir.change.subscribe(() => this._changeDetectorRef.markForCheck()); }