Skip to content
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.

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

feat(slide-toggle): add ripple focus indicator #3739

Merged
merged 2 commits into from
Mar 29, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 5 additions & 3 deletions src/lib/slide-toggle/index.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import {NgModule, ModuleWithProviders} from '@angular/core';
import {FormsModule} from '@angular/forms';
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
import {GestureConfig, CompatibilityModule} from '../core';
import {GestureConfig, CompatibilityModule, MdRippleModule, FocusOriginMonitor} from '../core';
import {MdSlideToggle} from './slide-toggle';
import {MdRippleModule} from '../core/ripple/index';


@NgModule({
imports: [FormsModule, MdRippleModule, CompatibilityModule],
exports: [MdSlideToggle, CompatibilityModule],
declarations: [MdSlideToggle],
providers: [{provide: HAMMER_GESTURE_CONFIG, useClass: GestureConfig}],
providers: [
FocusOriginMonitor,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be FOCUS_ORIGIN_MONITOR_PROVIDER

{ provide: HAMMER_GESTURE_CONFIG, useClass: GestureConfig }
],
})
export class MdSlideToggleModule {
/** @deprecated */
Expand Down
2 changes: 0 additions & 2 deletions src/lib/slide-toggle/slide-toggle.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
[attr.name]="name"
[attr.aria-label]="ariaLabel"
[attr.aria-labelledby]="ariaLabelledby"
(blur)="_onInputBlur()"
(focus)="_onInputFocus()"
(change)="_onChangeEvent($event)"
(click)="_onInputClick($event)">

Expand Down
32 changes: 21 additions & 11 deletions src/lib/slide-toggle/slide-toggle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {FormsModule, NgControl, ReactiveFormsModule, FormControl} from '@angular
import {MdSlideToggle, MdSlideToggleChange, MdSlideToggleModule} from './index';
import {TestGestureConfig} from '../slider/test-gesture-config';
import {dispatchFakeEvent} from '../core/testing/dispatch-events';
import {RIPPLE_FADE_IN_DURATION, RIPPLE_FADE_OUT_DURATION} from '../core/ripple/ripple-renderer';

describe('MdSlideToggle', () => {

Expand Down Expand Up @@ -268,6 +269,26 @@ describe('MdSlideToggle', () => {
fixture.detectChanges();
});

it('should show a ripple when focused by a keyboard action', fakeAsync(() => {
expect(slideToggleElement.querySelectorAll('.mat-ripple-element').length)
.toBe(0, 'Expected no ripples to be present.');

dispatchFakeEvent(inputElement, 'keydown');
dispatchFakeEvent(inputElement, 'focus');

tick(RIPPLE_FADE_IN_DURATION);

expect(slideToggleElement.querySelectorAll('.mat-ripple-element').length)
.toBe(1, 'Expected the focus ripple to be showing up.');

dispatchFakeEvent(inputElement, 'blur');

tick(RIPPLE_FADE_OUT_DURATION);

expect(slideToggleElement.querySelectorAll('.mat-ripple-element').length)
.toBe(0, 'Expected focus ripple to be removed.');
}));

it('should have the correct control state initially and after interaction', () => {
// The control should start off valid, pristine, and untouched.
expect(slideToggleControl.valid).toBe(true);
Expand Down Expand Up @@ -327,15 +348,6 @@ describe('MdSlideToggle', () => {
});
}));

it('should correctly set the slide-toggle to checked on focus', () => {
expect(slideToggleElement.classList).not.toContain('mat-slide-toggle-focused');

dispatchFakeEvent(inputElement, 'focus');
fixture.detectChanges();

expect(slideToggleElement.classList).toContain('mat-slide-toggle-focused');
});

it('should forward the required attribute', () => {
testComponent.isRequired = true;
fixture.detectChanges();
Expand All @@ -349,14 +361,12 @@ describe('MdSlideToggle', () => {
});

it('should focus on underlying input element when focus() is called', () => {
expect(slideToggleElement.classList).not.toContain('mat-slide-toggle-focused');
expect(document.activeElement).not.toBe(inputElement);

slideToggle.focus();
fixture.detectChanges();

expect(document.activeElement).toBe(inputElement);
expect(slideToggleElement.classList).toContain('mat-slide-toggle-focused');
});

it('should set a element class if labelPosition is set to before', () => {
Expand Down
79 changes: 55 additions & 24 deletions src/lib/slide-toggle/slide-toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@ import {
AfterContentInit,
ViewChild,
ViewEncapsulation,
OnDestroy,
} from '@angular/core';
import {
applyCssTransform,
coerceBooleanProperty,
HammerInput,
FocusOriginMonitor,
FocusOrigin,
MdRipple,
RippleRef
} from '../core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {applyCssTransform, coerceBooleanProperty, HammerInput} from '../core';
import {Observable} from 'rxjs/Observable';

import {Subscription} from 'rxjs/Subscription';

export const MD_SLIDE_TOGGLE_VALUE_ACCESSOR: any = {
provide: NG_VALUE_ACCESSOR,
Expand All @@ -41,8 +50,6 @@ let nextId = 0;
'[class.mat-slide-toggle]': 'true',
'[class.mat-checked]': 'checked',
'[class.mat-disabled]': 'disabled',
// This mat-slide-toggle prefix will change, once the temporary ripple is removed.
'[class.mat-slide-toggle-focused]': '_hasFocus',
'[class.mat-slide-toggle-label-before]': 'labelPosition == "before"',
'(mousedown)': '_setMousedown()'
},
Expand All @@ -52,7 +59,7 @@ let nextId = 0;
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
export class MdSlideToggle implements OnDestroy, AfterContentInit, ControlValueAccessor {

private onChange = (_: any) => {};
private onTouched = () => {};
Expand All @@ -67,8 +74,11 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
private _required: boolean = false;
private _disableRipple: boolean = false;

// Needs to be public to support AOT compilation (as host binding).
_hasFocus: boolean = false;
/** Reference to the focus state ripple. */
private _focusRipple: RippleRef;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_focusIndicator ?

Copy link
Member Author

@devversion devversion Mar 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really prefer _focusRipple here because it's just indicates that it's a ripple. On the checkbox we have it similar. Just focusedRipple.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way is fine; my thinking is that the "Ripple" part of the identity here is captured by the type.


/** Reference to the focus origin monitor subscription. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/** Subscription to focus-origin changes */

?

private _focusOriginSubscription: Subscription;

/** Name value will be applied to the input element if present */
@Input() name: string = null;
Expand Down Expand Up @@ -110,12 +120,31 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
/** Returns the unique id for the visual hidden input. */
get inputId(): string { return `${this.id || this._uniqueId}-input`; }

/** Reference to the underlying input element. */
@ViewChild('input') _inputElement: ElementRef;

constructor(private _elementRef: ElementRef, private _renderer: Renderer) {}
/** Reference to the ripple directive on the thumb container. */
@ViewChild(MdRipple) _ripple: MdRipple;

constructor(private _elementRef: ElementRef,
private _renderer: Renderer,
private _focusOriginMonitor: FocusOriginMonitor) {}

ngAfterContentInit() {
this._slideRenderer = new SlideToggleRenderer(this._elementRef);

this._focusOriginSubscription = this._focusOriginMonitor
.monitor(this._inputElement.nativeElement, this._renderer, false)
.subscribe(focusOrigin => this._onInputFocusChange(focusOrigin));
}

ngOnDestroy() {
this._focusOriginMonitor.unmonitor(this._inputElement.nativeElement);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe in another PR we can rename unmonitor to stopMonitoring...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I will note it down.


if (this._focusOriginSubscription) {
this._focusOriginSubscription.unsubscribe();
this._focusOriginSubscription = null;
}
}

/**
Expand Down Expand Up @@ -162,19 +191,6 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
setTimeout(() => this._isMousedown = false, 100);
}

_onInputFocus() {
// Only show the focus / ripple indicator when the focus was not triggered by a mouse
// interaction on the component.
if (!this._isMousedown) {
this._hasFocus = true;
}
}

_onInputBlur() {
this._hasFocus = false;
this.onTouched();
}

/** Implemented as part of ControlValueAccessor. */
writeValue(value: any): void {
this.checked = value;
Expand All @@ -195,10 +211,9 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
this.disabled = isDisabled;
}

/** Focuses the slide-toggle. */
/** Focuses the slide-toggle programmatically. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the "programmatically" is necessary

focus() {
this._renderer.invokeElementMethod(this._inputElement.nativeElement, 'focus');
this._onInputFocus();
this._focusOriginMonitor.focusVia(this._inputElement.nativeElement, this._renderer, 'program');
}

/** Whether the slide-toggle is checked. */
Expand All @@ -223,6 +238,22 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
this.checked = !this.checked;
}

/** Function is called whenever the focus changes for the input element. */
private _onInputFocusChange(focusOrigin: FocusOrigin) {
if (!this._focusRipple && focusOrigin === 'keyboard') {
// For keyboard focus show a persistent ripple as focus indicator.
this._focusRipple = this._ripple.launch(0, 0, { persistent: true, centered: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{persistent: true, centered: true}
(no leading/trailing spaces)

} else if (!focusOrigin) {
this.onTouched();

// Fade out and clear the focus ripple if one is currently present.
if (this._focusRipple) {
this._focusRipple.fadeOut();
this._focusRipple = null;
}
}
}

private _updateColor(newColor: string) {
this._setElementColor(this._color, false);
this._setElementColor(newColor, true);
Expand Down