Skip to content

Commit

Permalink
fix(material/input): inconsistently reading name from input with ngMo…
Browse files Browse the repository at this point in the history
…del (#19233)

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.

(cherry picked from commit 1926b19)
  • Loading branch information
crisbeto authored and andrewseguin committed Jan 14, 2022
1 parent 70e0170 commit feac08f
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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',
// Only mark the input as invalid for assistive technology if it has a value since the
Expand Down
7 changes: 7 additions & 0 deletions src/material/input/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const _MatInputBase = mixinErrorState(
'[attr.data-placeholder]': 'placeholder',
'[disabled]': 'disabled',
'[required]': 'required',
'[attr.name]': 'name || null',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[class.mat-native-select-inline]': '_isInlineSelect()',
// Only mark the input as invalid for assistive technology if it has a value since the
Expand Down Expand Up @@ -183,6 +184,12 @@ export class MatInput
*/
@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-input.spec.ts
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 {getSupportedInputTypes} from '@angular/cdk/platform';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
Expand All @@ -18,7 +18,7 @@ export function runInputHarnessTests(

beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [NoopAnimationsModule, inputModule, ReactiveFormsModule],
imports: [NoopAnimationsModule, inputModule, FormsModule],
declarations: [InputHarnessTest],
}).compileComponents();

Expand All @@ -29,7 +29,7 @@ export function runInputHarnessTests(

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

it('should load input with specific id', async () => {
Expand Down Expand Up @@ -68,37 +68,40 @@ export function runInputHarnessTests(

it('should be able to get id of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(6);
expect(inputs.length).toBe(7);
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(6);
expect(inputs.length).toBe(7);
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(6);
expect(inputs.length).toBe(7);
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(6);
expect(inputs.length).toBe(7);
expect(await inputs[0].getValue()).toBe('Sushi');
expect(await inputs[1].getValue()).toBe('');
expect(await inputs[3].getValue()).toBe('');
Expand All @@ -117,13 +120,14 @@ export function runInputHarnessTests(

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

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 @@ -132,13 +136,14 @@ export function runInputHarnessTests(

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

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 @@ -147,13 +152,14 @@ export function runInputHarnessTests(

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

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 @@ -162,22 +168,24 @@ export function runInputHarnessTests(

it('should be able to get placeholder of input', async () => {
const inputs = await loader.getAllHarnesses(inputHarness);
expect(inputs.length).toBe(6);
expect(inputs.length).toBe(7);
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(6);
expect(inputs.length).toBe(7);
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 @@ -248,6 +256,10 @@ export function runInputHarnessTests(
</select>
</mat-form-field>
<mat-form-field>
<input [(ngModel)]="ngModelValue" [name]="ngModelName" id="has-ng-model" matNativeControl>
</mat-form-field>
<mat-form-field>
<input matNativeControl placeholder="Color control" id="colorControl" type="color">
</mat-form-field>
Expand All @@ -258,4 +270,6 @@ class InputHarnessTest {
readonly = false;
disabled = false;
required = false;
ngModelValue = '';
ngModelName = 'has-ng-model';
}
3 changes: 2 additions & 1 deletion tools/public_api_guard/material/input.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class MatInput extends _MatInputBase implements MatFormFieldControl<any>,
protected _isNeverEmpty(): boolean;
readonly _isServer: boolean;
readonly _isTextarea: boolean;
name: string;
// (undocumented)
protected _neverEmptyInputTypes: string[];
// (undocumented)
Expand Down Expand Up @@ -103,7 +104,7 @@ export class MatInput extends _MatInputBase implements MatFormFieldControl<any>,
get value(): string;
set value(value: any);
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<MatInput, "input[matInput], textarea[matInput], select[matNativeControl], input[matNativeControl], textarea[matNativeControl]", ["matInput"], { "disabled": "disabled"; "id": "id"; "placeholder": "placeholder"; "required": "required"; "type": "type"; "errorStateMatcher": "errorStateMatcher"; "userAriaDescribedBy": "aria-describedby"; "value": "value"; "readonly": "readonly"; }, {}, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<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"; "userAriaDescribedBy": "aria-describedby"; "value": "value"; "readonly": "readonly"; }, {}, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatInput, [null, null, { optional: true; self: true; }, { optional: true; }, { optional: true; }, null, { optional: true; self: true; }, null, null, { optional: true; }]>;
}
Expand Down

0 comments on commit feac08f

Please sign in to comment.