Skip to content

Commit

Permalink
fix(input/testing): inconsistently reading name from input with ngModel
Browse files Browse the repository at this point in the history 
If an input has a `name` binding and an `ngModel`, the input harness won't be able to read the name from the DOM, because `ngModel` doesn't proxy it. These changes add the proxy behavior to the `MatInput` directive, similarly to what we we're doing for `required`, `placeholder`, `readonly` etc.

Fixes #18624.
  • Loading branch information
@crisbeto
crisbeto committed May 2, 2020
1 parent 658896f commit f3342aa
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/material-experimental/mdc-input/input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {MatInput as BaseMatInput} from '@angular/material/input';
'[id]': 'id',
'[disabled]': 'disabled',
'[required]': 'required',
'[attr.name]': 'name',
'[attr.placeholder]': 'placeholder',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[attr.aria-describedby]': '_ariaDescribedby || null',
Expand Down
7 changes: 7 additions & 0 deletions src/material/input/input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase =
'[attr.placeholder]': 'placeholder',
'[disabled]': 'disabled',
'[required]': 'required',
'[attr.name]': 'name || null',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[attr.aria-describedby]': '_ariaDescribedby || null',
'[attr.aria-invalid]': 'errorState',
Expand Down Expand Up @@ -163,6 +164,12 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
*/
@Input() placeholder: string;

/**
* Name of the input.
* @docs-private
*/
@Input() name: string;

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
Expand Down
38 changes: 26 additions & 12 deletions src/material/input/testing/shared.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {HarnessLoader} from '@angular/cdk/testing';
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
import {Component} from '@angular/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {ReactiveFormsModule} from '@angular/forms';
import {FormsModule} from '@angular/forms';
import {MatInputModule} from '@angular/material/input';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {MatInputHarness} from './input-harness';
Expand All @@ -16,7 +16,7 @@ export function runHarnessTests(
beforeEach(async () => {
await TestBed
.configureTestingModule({
imports: [NoopAnimationsModule, inputModule, ReactiveFormsModule],
imports: [NoopAnimationsModule, inputModule, FormsModule],
declarations: [InputHarnessTest],
})
.compileComponents();
Expand All @@ -28,7 +28,7 @@ export function runHarnessTests(

it('should load all input harnesses', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
});

it('should load input with specific id', async () => {
Expand All @@ -49,37 +49,40 @@ export function runHarnessTests(

it('should be able to get id of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getId()).toMatch(/mat-input-\d+/);
expect(await inputs[1].getId()).toMatch(/mat-input-\d+/);
expect(await inputs[2].getId()).toBe('myTextarea');
expect(await inputs[3].getId()).toBe('nativeControl');
expect(await inputs[4].getId()).toMatch(/mat-input-\d+/);
expect(await inputs[5].getId()).toBe('has-ng-model');
});

it('should be able to get name of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getName()).toBe('favorite-food');
expect(await inputs[1].getName()).toBe('');
expect(await inputs[2].getName()).toBe('');
expect(await inputs[3].getName()).toBe('');
expect(await inputs[4].getName()).toBe('');
expect(await inputs[5].getName()).toBe('has-ng-model');
});

it('should be able to get value of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getValue()).toBe('Sushi');
expect(await inputs[1].getValue()).toBe('');
expect(await inputs[2].getValue()).toBe('');
expect(await inputs[3].getValue()).toBe('');
expect(await inputs[4].getValue()).toBe('');
expect(await inputs[5].getValue()).toBe('');
});

it('should be able to set value of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getValue()).toBe('Sushi');
expect(await inputs[1].getValue()).toBe('');
expect(await inputs[3].getValue()).toBe('');
Expand All @@ -98,13 +101,14 @@ export function runHarnessTests(

it('should be able to get disabled state', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);

expect(await inputs[0].isDisabled()).toBe(false);
expect(await inputs[1].isDisabled()).toBe(false);
expect(await inputs[2].isDisabled()).toBe(false);
expect(await inputs[3].isDisabled()).toBe(false);
expect(await inputs[4].isDisabled()).toBe(false);
expect(await inputs[5].isDisabled()).toBe(false);

fixture.componentInstance.disabled = true;

Expand All @@ -113,13 +117,14 @@ export function runHarnessTests(

it('should be able to get readonly state', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);

expect(await inputs[0].isReadonly()).toBe(false);
expect(await inputs[1].isReadonly()).toBe(false);
expect(await inputs[2].isReadonly()).toBe(false);
expect(await inputs[3].isReadonly()).toBe(false);
expect(await inputs[4].isReadonly()).toBe(false);
expect(await inputs[5].isReadonly()).toBe(false);

fixture.componentInstance.readonly = true;

Expand All @@ -128,13 +133,14 @@ export function runHarnessTests(

it('should be able to get required state', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);

expect(await inputs[0].isRequired()).toBe(false);
expect(await inputs[1].isRequired()).toBe(false);
expect(await inputs[2].isRequired()).toBe(false);
expect(await inputs[3].isRequired()).toBe(false);
expect(await inputs[4].isRequired()).toBe(false);
expect(await inputs[5].isRequired()).toBe(false);

fixture.componentInstance.required = true;

Expand All @@ -143,22 +149,24 @@ export function runHarnessTests(

it('should be able to get placeholder of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getPlaceholder()).toBe('Favorite food');
expect(await inputs[1].getPlaceholder()).toBe('');
expect(await inputs[2].getPlaceholder()).toBe('Leave a comment');
expect(await inputs[3].getPlaceholder()).toBe('Native control');
expect(await inputs[4].getPlaceholder()).toBe('');
expect(await inputs[5].getPlaceholder()).toBe('');
});

it('should be able to get type of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(5);
expect(inputs.length).toBe(6);
expect(await inputs[0].getType()).toBe('text');
expect(await inputs[1].getType()).toBe('number');
expect(await inputs[2].getType()).toBe('textarea');
expect(await inputs[3].getType()).toBe('text');
expect(await inputs[4].getType()).toBe('textarea');
expect(await inputs[5].getType()).toBe('text');

fixture.componentInstance.inputType = 'text';

Expand Down Expand Up @@ -220,11 +228,17 @@ function getActiveElementTagName() {
<option value="first">First</option>
</select>
</mat-form-field>
<mat-form-field>
<input [(ngModel)]="ngModelValue" [name]="ngModelName" id="has-ng-model" matNativeControl>
</mat-form-field>
`
})
class InputHarnessTest {
inputType = 'number';
readonly = false;
disabled = false;
required = false;
ngModelValue = '';
ngModelName = 'has-ng-model';
}
3 changes: 2 additions & 1 deletion tools/public_api_guard/material/input.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField
focused: boolean;
get id(): string;
set id(value: string);
name: string;
ngControl: NgControl;
placeholder: string;
get readonly(): boolean;
Expand Down Expand Up @@ -58,7 +59,7 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField
static ngAcceptInputType_readonly: BooleanInput;
static ngAcceptInputType_required: BooleanInput;
static ngAcceptInputType_value: any;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatInput, "input[matInput], textarea[matInput], select[matNativeControl], input[matNativeControl], textarea[matNativeControl]", ["matInput"], { "disabled": "disabled"; "id": "id"; "placeholder": "placeholder"; "required": "required"; "type": "type"; "errorStateMatcher": "errorStateMatcher"; "value": "value"; "readonly": "readonly"; }, {}, never>;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatInput, "input[matInput], textarea[matInput], select[matNativeControl], input[matNativeControl], textarea[matNativeControl]", ["matInput"], { "disabled": "disabled"; "id": "id"; "placeholder": "placeholder"; "name": "name"; "required": "required"; "type": "type"; "errorStateMatcher": "errorStateMatcher"; "value": "value"; "readonly": "readonly"; }, {}, never>;
static ɵfac: i0.ɵɵFactoryDef<MatInput, [null, null, { optional: true; self: true; }, { optional: true; }, { optional: true; }, null, { optional: true; self: true; }, null, null]>;
}

Expand Down

0 comments on commit f3342aa

Please sign in to comment.