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

fix(slide-toggle): invalid model change event #4220

Merged
merged 3 commits into from
Jul 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 38 additions & 10 deletions src/lib/slide-toggle/slide-toggle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,18 +547,21 @@ describe('MdSlideToggle with forms', () => {
expect(slideToggleModel.pristine).toBe(true);
expect(slideToggleModel.touched).toBe(false);

// After changing the value programmatically, the control should
// become dirty (not pristine), but remain untouched.
// After changing the value from the view, the control should
// become dirty (not pristine), but remain untouched if focus is still there.
slideToggle.checked = true;
fixture.detectChanges();

// Dispatch a change event on the input element to fake a user interaction that triggered
// the state change.
dispatchFakeEvent(inputElement, 'change');

expect(slideToggleModel.valid).toBe(true);
expect(slideToggleModel.pristine).toBe(false);
expect(slideToggleModel.touched).toBe(false);

// After a user interaction occurs (such as a click), the control should remain dirty and
// now also be touched.
labelElement.click();
// Once the input element loses focus, the control should remain dirty but should
// also turn touched.
dispatchFakeEvent(inputElement, 'blur');
fixture.detectChanges();

expect(slideToggleModel.valid).toBe(true);
Expand All @@ -576,13 +579,13 @@ describe('MdSlideToggle with forms', () => {
expect(slideToggleModel.touched).toBe(false);
expect(slideToggleElement.classList).toContain('mat-checked');

// After a user interaction occurs (such as a click), the control should remain dirty and
// now also be touched.
inputElement.click();
// Once the input element loses focus, the control should remain dirty but should
// also turn touched.
dispatchFakeEvent(inputElement, 'blur');
fixture.detectChanges();

expect(slideToggleModel.touched).toBe(true);
expect(slideToggleElement.classList).not.toContain('mat-checked');
expect(slideToggleElement.classList).toContain('mat-checked');
});

it('should not set the control to touched when changing the model', fakeAsync(() => {
Expand All @@ -603,6 +606,31 @@ describe('MdSlideToggle with forms', () => {
expect(slideToggle.checked).toBe(true);
expect(slideToggleElement.classList).toContain('mat-checked');
}));

it('should update checked state on click if control is checked initially', fakeAsync(() => {
fixture = TestBed.createComponent(SlideToggleWithModel);
slideToggle = fixture.debugElement.query(By.directive(MdSlideToggle)).componentInstance;
labelElement = fixture.debugElement.query(By.css('label')).nativeElement;

fixture.componentInstance.modelValue = true;
fixture.detectChanges();

// Flush the microtasks because the forms module updates the model state asynchronously.
flushMicrotasks();

// Now the new checked variable has been updated in the slide-toggle and the slide-toggle
// is marked for check because it still needs to update the underlying input.
fixture.detectChanges();

expect(slideToggle.checked)
.toBe(true, 'Expected slide-toggle to be checked initially');

labelElement.click();
fixture.detectChanges();

expect(slideToggle.checked)
.toBe(false, 'Expected slide-toggle to be no longer checked after label click.');
}));
});

describe('with a FormControl', () => {
Expand Down
54 changes: 28 additions & 26 deletions src/lib/slide-toggle/slide-toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
private onTouched = () => {};

private _uniqueId: string = `md-slide-toggle-${++nextUniqueId}`;
private _checked: boolean = false;
private _slideRenderer: SlideToggleRenderer;
private _required: boolean = false;
private _checked: boolean = false;

/** Reference to the focus state ripple. */
private _focusRipple: RippleRef | null;
Expand All @@ -103,6 +103,8 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
/** Whether the label should appear after or before the slide-toggle. Defaults to 'after' */
@Input() labelPosition: 'before' | 'after' = 'after';

/** Whether the slide-toggle element is checked or not */

/** Used to set the aria-label attribute on the underlying input element. */
@Input('aria-label') ariaLabel: string | null = null;

Expand All @@ -114,6 +116,13 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
get required(): boolean { return this._required; }
set required(value) { this._required = coerceBooleanProperty(value); }

/** Whether the slide-toggle element is checked or not */
@Input()
get checked(): boolean { return this._checked; }
set checked(value) {
this._checked = !!value;
this._changeDetectorRef.markForCheck();
}
/** An event will be dispatched each time the slide-toggle changes its value. */
@Output() change: EventEmitter<MdSlideToggleChange> = new EventEmitter<MdSlideToggleChange>();

Expand Down Expand Up @@ -147,29 +156,30 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
}

/**
* The onChangeEvent method will be also called on click.
* This is because everything for the slide-toggle is wrapped inside of a label,
* which triggers a onChange event on click.
* This function will called if the underlying input changed its value through user interaction.
*/
_onChangeEvent(event: Event) {
// We always have to stop propagation on the change event.
// Otherwise the change event, from the input element, will bubble up and
// emit its event object to the component's `change` output.
event.stopPropagation();

// Once a drag is currently in progress, we do not want to toggle the slide-toggle on a click.
if (!this.disabled && !this._slideRenderer.dragging) {
this.toggle();
// Sync the value from the underlying input element with the slide-toggle component.
this.checked = this._inputElement.nativeElement.checked;

// Emit our custom change event if the native input emitted one.
// It is important to only emit it, if the native input triggered one, because
// we don't want to trigger a change event, when the `checked` variable changes for example.
this._emitChangeEvent();
}
// Emit our custom change event if the native input emitted one.
// It is important to only emit it, if the native input triggered one, because we don't want
// to trigger a change event, when the `checked` variable changes programmatically.
this._emitChangeEvent();
}

_onInputClick(event: Event) {
this.onTouched();
// In some situations the user will release the mouse on the label element. The label element
// redirects the click to the underlying input element and will result in a value change.
// Prevent the default behavior if dragging, because the value will be set after drag.
if (this._slideRenderer.dragging) {
event.preventDefault();
}

// We have to stop propagation for click events on the visual hidden input element.
// By default, when a user clicks on a label element, a generated click event will be
Expand All @@ -183,7 +193,7 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,

/** Implemented as part of ControlValueAccessor. */
writeValue(value: any): void {
this.checked = value;
this.checked = !!value;
}

/** Implemented as part of ControlValueAccessor. */
Expand All @@ -207,16 +217,6 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
this._focusOriginMonitor.focusVia(this._inputElement.nativeElement, 'keyboard');
}

/** Whether the slide-toggle is checked. */
@Input()
get checked() { return !!this._checked; }
set checked(value) {
if (this.checked !== !!value) {
this._checked = value;
this.onChange(this._checked);
}
}

/** Toggles the checked state of the slide-toggle. */
toggle() {
this.checked = !this.checked;
Expand All @@ -238,15 +238,17 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
}
}

/** Emits the change event to the `change` output EventEmitter */
/**
* Emits a change event on the `change` output. Also notifies the FormControl about the change.
*/
private _emitChangeEvent() {
let event = new MdSlideToggleChange();
event.source = this;
event.checked = this.checked;
this.change.emit(event);
this.onChange(this.checked);
}


_onDragStart() {
if (!this.disabled) {
this._slideRenderer.startThumbDrag(this.checked);
Expand Down