Skip to content

Commit

Permalink
fix(input): properly determine input value
Browse files Browse the repository at this point in the history
Right now the `MdInputDirective` tries to cache the `value` of the input.

To do this, the `MdInputDirective` needs to listen for `NgControl` value changes and for native `(change)` events.

This will be problematic when a value is set directly to the input element (using `[value]` property binding) because Angular is only able to recognize this change in the first ChangeDetection.

```html
<md-input-container>
  <input md-input [value]="myValue" placeholder="Label">
</md-input-container>
```

The approach of updating the value in the `ngAfterViewInit` lifecycle hook, will result in a binding change while being in a ChangeDetection, which leads to a ChangeDetection error.

```
Expression has changed after it was checked. Previous value: 'true'. Current value: 'false'.
```

Fixes angular#2441. Fixes angular#2363
  • Loading branch information
devversion committed Dec 29, 2016
1 parent dccbe41 commit e8d558d
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 18 deletions.
51 changes: 49 additions & 2 deletions src/lib/input/input-container.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {async, TestBed, inject} from '@angular/core/testing';
import {Component} from '@angular/core';
import {FormsModule, ReactiveFormsModule} from '@angular/forms';
import {FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
import {By} from '@angular/platform-browser';
import {MdInputModule} from './input';
import {MdInputContainer} from './input-container';
import {MdInputContainer, MdInputDirective} from './input-container';
import {Platform} from '../core/platform/platform';
import {PlatformModule} from '../core/platform/index';
import {
Expand Down Expand Up @@ -41,6 +41,8 @@ describe('MdInputContainer', function () {
MdInputContainerZeroTestController,
MdTextareaWithBindings,
MdInputContainerWithDisabled,
MdInputContainerWithValueBinding,
MdInputContainerWithFormControl,
MdInputContainerMissingMdInputTestController
],
});
Expand Down Expand Up @@ -128,6 +130,20 @@ describe('MdInputContainer', function () {
expect(el.classList.contains('md-empty')).toBe(false, 'should not be empty');
}));

it('should not be empty when the value set before view init', async(() => {
let fixture = TestBed.createComponent(MdInputContainerWithValueBinding);
fixture.detectChanges();

let placeholderEl = fixture.debugElement.query(By.css('.md-input-placeholder')).nativeElement;

expect(placeholderEl.classList).not.toContain('md-empty');

fixture.componentInstance.value = '';
fixture.detectChanges();

expect(placeholderEl.classList).toContain('md-empty');
}));

it('should not treat the number 0 as empty', async(() => {
let fixture = TestBed.createComponent(MdInputContainerZeroTestController);
fixture.detectChanges();
Expand All @@ -141,6 +157,20 @@ describe('MdInputContainer', function () {
});
}));

it('should update the value when using FormControl.setValue', () => {
let fixture = TestBed.createComponent(MdInputContainerWithFormControl);
fixture.detectChanges();

let input = fixture.debugElement.query(By.directive(MdInputDirective))
.injector.get(MdInputDirective) as MdInputDirective;

expect(input.value).toBeFalsy();

fixture.componentInstance.formControl.setValue('something');

expect(input.value).toBe('something');
});

it('should add id', () => {
let fixture = TestBed.createComponent(MdInputContainerTextTestController);
fixture.detectChanges();
Expand Down Expand Up @@ -326,6 +356,13 @@ class MdInputContainerPlaceholderElementTestComponent {
placeholder: string = 'Default Placeholder';
}

@Component({
template: `<md-input-container><input md-input [formControl]="formControl"></md-input-container>`
})
class MdInputContainerWithFormControl {
formControl = new FormControl();
}

@Component({
template: `<md-input-container><input md-input [placeholder]="placeholder"></md-input-container>`
})
Expand Down Expand Up @@ -429,6 +466,16 @@ class MdInputContainerZeroTestController {
value = 0;
}

@Component({
template: `
<md-input-container>
<input md-input placeholder="Label" [value]="value">
</md-input-container>`
})
class MdInputContainerWithValueBinding {
value: string = 'Initial';
}

@Component({
template: `
<md-input-container>
Expand Down
22 changes: 6 additions & 16 deletions src/lib/input/input-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ export class MdHint {
'[id]': 'id',
'(blur)': '_onBlur()',
'(focus)': '_onFocus()',
'(input)': '_onInput()',
}
})
export class MdInputDirective implements AfterContentInit {
export class MdInputDirective {

/** Whether the element is disabled. */
@Input()
get disabled() { return this._disabled; }
Expand Down Expand Up @@ -116,8 +116,9 @@ export class MdInputDirective implements AfterContentInit {
}
private _type = 'text';

/** The element's value. */
value: any;
/** The input element's value. */
get value() { return this._elementRef.nativeElement.value; }
set value(value: string) { this._elementRef.nativeElement.value = value; }

/**
* Emits an event when the placeholder changes so that the `md-input-container` can re-validate.
Expand All @@ -143,18 +144,9 @@ export class MdInputDirective implements AfterContentInit {
constructor(private _elementRef: ElementRef,
private _renderer: Renderer,
@Optional() public _ngControl: NgControl) {

// Force setter to be called in case id was not specified.
this.id = this.id;

if (this._ngControl && this._ngControl.valueChanges) {
this._ngControl.valueChanges.subscribe((value) => {
this.value = value;
});
}
}

ngAfterContentInit() {
this.value = this._elementRef.nativeElement.value;
}

/** Focuses the input element. */
Expand All @@ -164,8 +156,6 @@ export class MdInputDirective implements AfterContentInit {

_onBlur() { this.focused = false; }

_onInput() { this.value = this._elementRef.nativeElement.value; }

/** Make sure the input is a supported type. */
private _validateType() {
if (MD_INPUT_INVALID_TYPES.indexOf(this._type) != -1) {
Expand Down

0 comments on commit e8d558d

Please sign in to comment.