From 400c6c8cc17194b98ae8eda691cc7c4ab6a73c4e Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 27 Dec 2016 21:45:52 +0100 Subject: [PATCH 1/3] fix(input): ensure that property bindings work * Developers are currently not able to use property bindings for `required`, `disabled`, `type` and `placeholder`. * This commit ensures that the Angular input setters are in sync with the native input element. * Adds test to ensure that the property bindings work for the native input element (and not for the component instance as before) Fixes #2428 --- src/lib/input/input-container.spec.ts | 67 ++++++++++++++++++++++++--- src/lib/input/input-container.ts | 53 ++++++++++++++------- 2 files changed, 97 insertions(+), 23 deletions(-) diff --git a/src/lib/input/input-container.spec.ts b/src/lib/input/input-container.spec.ts index f94227041189..93512f0b857f 100644 --- a/src/lib/input/input-container.spec.ts +++ b/src/lib/input/input-container.spec.ts @@ -41,6 +41,8 @@ describe('MdInputContainer', function () { MdInputContainerZeroTestController, MdTextareaWithBindings, MdInputContainerWithDisabled, + MdInputContainerWithRequired, + MdInputContainerWithType, MdInputContainerMissingMdInputTestController ], }); @@ -236,16 +238,20 @@ describe('MdInputContainer', function () { let fixture = TestBed.createComponent(MdInputContainerPlaceholderAttrTestComponent); fixture.detectChanges(); - let el = fixture.debugElement.query(By.css('label')); - expect(el).toBeNull(); + let inputEl = fixture.debugElement.query(By.css('input')).nativeElement; + + expect(fixture.debugElement.query(By.css('label'))).toBeNull(); + expect(inputEl.placeholder).toBe(''); fixture.componentInstance.placeholder = 'Other placeholder'; fixture.detectChanges(); - el = fixture.debugElement.query(By.css('label')); - expect(el).not.toBeNull(); - expect(el.nativeElement.textContent).toMatch('Other placeholder'); - expect(el.nativeElement.textContent).not.toMatch(/\*/g); + let labelEl = fixture.debugElement.query(By.css('label')); + + expect(inputEl.placeholder).toBe('Other placeholder'); + expect(labelEl).not.toBeNull(); + expect(labelEl.nativeElement.textContent).toMatch('Other placeholder'); + expect(labelEl.nativeElement.textContent).not.toMatch(/\*/g); })); it('supports placeholder element', async(() => { @@ -274,18 +280,51 @@ describe('MdInputContainer', function () { expect(el.nativeElement.textContent).toMatch(/hello\s+\*/g); }); - it('supports the disabled attribute', async(() => { + it('supports the disabled attribute as binding', async(() => { let fixture = TestBed.createComponent(MdInputContainerWithDisabled); fixture.detectChanges(); let underlineEl = fixture.debugElement.query(By.css('.md-input-underline')).nativeElement; + let inputEl = fixture.debugElement.query(By.css('input')).nativeElement; + expect(underlineEl.classList.contains('md-disabled')).toBe(false, 'should not be disabled'); + expect(inputEl.disabled).toBe(false); fixture.componentInstance.disabled = true; fixture.detectChanges(); + + expect(inputEl.disabled).toBe(true); expect(underlineEl.classList.contains('md-disabled')).toBe(true, 'should be disabled'); })); + it('supports the required attribute as binding', async(() => { + let fixture = TestBed.createComponent(MdInputContainerWithRequired); + fixture.detectChanges(); + + let inputEl = fixture.debugElement.query(By.css('input')).nativeElement; + + expect(inputEl.required).toBe(false); + + fixture.componentInstance.required = true; + fixture.detectChanges(); + + expect(inputEl.required).toBe(true); + })); + + it('supports the type attribute as binding', async(() => { + let fixture = TestBed.createComponent(MdInputContainerWithType); + fixture.detectChanges(); + + let inputEl = fixture.debugElement.query(By.css('input')).nativeElement; + + expect(inputEl.type).toBe('text'); + + fixture.componentInstance.type = 'password'; + fixture.detectChanges(); + + expect(inputEl.type).toBe('password'); + })); + it('supports textarea', () => { let fixture = TestBed.createComponent(MdTextareaWithBindings); fixture.detectChanges(); @@ -310,6 +349,20 @@ class MdInputContainerWithDisabled { disabled: boolean; } +@Component({ + template: `` +}) +class MdInputContainerWithRequired { + required: boolean; +} + +@Component({ + template: `` +}) +class MdInputContainerWithType { + type: string; +} + @Component({ template: `` }) diff --git a/src/lib/input/input-container.ts b/src/lib/input/input-container.ts index 58b80dc72846..560030b1fc42 100644 --- a/src/lib/input/input-container.ts +++ b/src/lib/input/input-container.ts @@ -75,41 +75,56 @@ export class MdHint { `, host: { 'class': 'md-input-element', + // Native input properties that are overwritten by Angular inputs need to be synced with + // the native input element. Otherwise property bindings for those don't work. '[id]': 'id', + '[placeholder]': 'placeholder', + '[disabled]': 'disabled', + '[required]': 'required', '(blur)': '_onBlur()', '(focus)': '_onFocus()', '(input)': '_onInput()', } }) export class MdInputDirective implements AfterContentInit { + + /** Variables used as cache for getters and setters. */ + private _type = 'text'; + private _placeholder: string = ''; + private _disabled = false; + private _required = false; + private _id: string; + private _cachedUid: string; + + /** The element's value. */ + value: any; + + /** Whether the element is focused or not. */ + focused = false; + /** Whether the element is disabled. */ @Input() get disabled() { return this._disabled; } set disabled(value: any) { this._disabled = coerceBooleanProperty(value); } - private _disabled = false; /** Unique id of the element. */ @Input() get id() { return this._id; }; - set id(value: string) { this._id = value || this._uid; } - private _id: string; + set id(value: string) {this._id = value || this._uid; } /** Placeholder attribute of the element. */ @Input() get placeholder() { return this._placeholder; } set placeholder(value: string) { - if (this._placeholder != value) { + if (this._placeholder !== value) { this._placeholder = value; this._placeholderChange.emit(this._placeholder); } } - private _placeholder = ''; - /** Whether the element is required. */ @Input() get required() { return this._required; } set required(value: any) { this._required = coerceBooleanProperty(value); } - private _required = false; /** Input type of the element. */ @Input() @@ -117,11 +132,14 @@ export class MdInputDirective implements AfterContentInit { set type(value: string) { this._type = value || 'text'; this._validateType(); - } - private _type = 'text'; - /** The element's value. */ - value: any; + // When using Angular inputs, developers are no longer able to set the properties on the native + // input element. To ensure that bindings for `type` work, we need to sync the setter + // with the native property. Textarea elements don't support the type property or attribute. + if (!this._isTextarea()) { + this._renderer.setElementProperty(this._elementRef.nativeElement, 'type', this._type); + } + } /** * Emits an event when the placeholder changes so that the `md-input-container` can re-validate. @@ -130,10 +148,7 @@ export class MdInputDirective implements AfterContentInit { get empty() { return (this.value == null || this.value === '') && !this._isNeverEmpty(); } - focused = false; - private get _uid() { return this._cachedUid = this._cachedUid || `md-input-${nextUniqueId++}`; } - private _cachedUid: string; private _neverEmptyInputTypes = [ 'date', @@ -172,12 +187,18 @@ export class MdInputDirective implements AfterContentInit { /** Make sure the input is a supported type. */ private _validateType() { - if (MD_INPUT_INVALID_TYPES.indexOf(this._type) != -1) { + if (MD_INPUT_INVALID_TYPES.indexOf(this._type) !== -1) { throw new MdInputContainerUnsupportedTypeError(this._type); } } - private _isNeverEmpty() { return this._neverEmptyInputTypes.indexOf(this._type) != -1; } + private _isNeverEmpty() { return this._neverEmptyInputTypes.indexOf(this._type) !== -1; } + + /** Determines if the component host is a textarea. If not recognizable it returns false. */ + private _isTextarea() { + let nativeElement = this._elementRef.nativeElement; + return nativeElement ? nativeElement.nodeName === 'textarea' : 'input'; + } } From 9ba74fb8eb788860469c9b054d8cbe8ec0cbdc3f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 27 Dec 2016 22:26:00 +0100 Subject: [PATCH 2/3] No longer set an unsupported type to the input element (IE11) --- src/lib/input/input-container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/input/input-container.ts b/src/lib/input/input-container.ts index 560030b1fc42..64f2c44e7f43 100644 --- a/src/lib/input/input-container.ts +++ b/src/lib/input/input-container.ts @@ -136,7 +136,7 @@ export class MdInputDirective implements AfterContentInit { // When using Angular inputs, developers are no longer able to set the properties on the native // input element. To ensure that bindings for `type` work, we need to sync the setter // with the native property. Textarea elements don't support the type property or attribute. - if (!this._isTextarea()) { + if (!this._isTextarea() && getSupportedInputTypes().has(this._type)) { this._renderer.setElementProperty(this._elementRef.nativeElement, 'type', this._type); } } From e4c9c511523d0e3a85523af78dba14ac13dbefef Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 27 Dec 2016 22:46:50 +0100 Subject: [PATCH 3/3] Address feedback --- src/lib/input/input-container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/input/input-container.ts b/src/lib/input/input-container.ts index 64f2c44e7f43..a74a52af9a7d 100644 --- a/src/lib/input/input-container.ts +++ b/src/lib/input/input-container.ts @@ -197,7 +197,7 @@ export class MdInputDirective implements AfterContentInit { /** Determines if the component host is a textarea. If not recognizable it returns false. */ private _isTextarea() { let nativeElement = this._elementRef.nativeElement; - return nativeElement ? nativeElement.nodeName === 'textarea' : 'input'; + return nativeElement ? nativeElement.nodeName.toLowerCase() === 'textarea' : false; } }