Skip to content

Commit

Permalink
fix(material-experimental/mdc-chips): use MDC-based form field (#20880)
Browse files Browse the repository at this point in the history
* Changes the MDC chips module to depend on the MDC form field module since we generally
don't support mixing MDC and non-MDC components.
* Fixes that the MDC chip input didn't work inside of an MDC form field.
* Switches the MDC chips demo to use mostly MDC components.
* Fixes some tests that broke after the switch to the MDC form field.
  • Loading branch information
crisbeto committed Nov 4, 2020
1 parent 5b1660e commit 7596532
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 46 deletions.
9 changes: 5 additions & 4 deletions src/dev-app/mdc-chips/BUILD.bazel
Expand Up @@ -10,11 +10,12 @@ ng_module(
":mdc_chips_demo_scss",
],
deps = [
"//src/material-experimental/mdc-button",
"//src/material-experimental/mdc-card",
"//src/material-experimental/mdc-checkbox",
"//src/material-experimental/mdc-chips",
"//src/material/button",
"//src/material/card",
"//src/material/checkbox",
"//src/material/form-field",
"//src/material-experimental/mdc-core",
"//src/material-experimental/mdc-form-field",
"//src/material/icon",
"//src/material/toolbar",
"@npm//@angular/router",
Expand Down
10 changes: 5 additions & 5 deletions src/dev-app/mdc-chips/mdc-chips-demo-module.ts
Expand Up @@ -9,12 +9,12 @@
import {CommonModule} from '@angular/common';
import {NgModule} from '@angular/core';
import {FormsModule, ReactiveFormsModule} from '@angular/forms';
import {MatButtonModule} from '@angular/material/button';
import {MatCardModule} from '@angular/material/card';
import {MatCheckboxModule} from '@angular/material/checkbox';
import {MatFormFieldModule} from '@angular/material/form-field';
import {MatToolbarModule} from '@angular/material/toolbar';
import {MatButtonModule} from '@angular/material-experimental/mdc-button';
import {MatCardModule} from '@angular/material-experimental/mdc-card';
import {MatCheckboxModule} from '@angular/material-experimental/mdc-checkbox';
import {MatFormFieldModule} from '@angular/material-experimental/mdc-form-field';
import {MatChipsModule} from '@angular/material-experimental/mdc-chips';
import {MatToolbarModule} from '@angular/material/toolbar';
import {MatIconModule} from '@angular/material/icon';
import {RouterModule} from '@angular/router';
import {MdcChipsDemo} from './mdc-chips-demo';
Expand Down
6 changes: 3 additions & 3 deletions src/dev-app/mdc-chips/mdc-chips-demo.html
Expand Up @@ -135,6 +135,7 @@ <h4>Multi selection</h4>
<h4>Input is last child of chip grid</h4>

<mat-form-field class="demo-has-chip-list">
<mat-label>New Contributor...</mat-label>
<mat-chip-grid #chipGrid1 [(ngModel)]="selectedPeople" required [disabled]="disableInputs">
<mat-chip-row *ngFor="let person of people"
[editable]="editable"
Expand All @@ -144,7 +145,6 @@ <h4>Input is last child of chip grid</h4>
<mat-icon matChipRemove>cancel</mat-icon>
</mat-chip-row>
<input [disabled]="disableInputs"
placeholder="New Contributor..."
[matChipInputFor]="chipGrid1"
[matChipInputSeparatorKeyCodes]="separatorKeysCodes"
[matChipInputAddOnBlur]="addOnBlur"
Expand All @@ -155,14 +155,14 @@ <h4>Input is last child of chip grid</h4>
<h4>Input is next sibling child of chip grid</h4>

<mat-form-field>
<mat-label>New Contributor...</mat-label>
<mat-chip-grid #chipGrid2 [(ngModel)]="selectedPeople" required [disabled]="disableInputs">
<mat-chip-row *ngFor="let person of people" (removed)="remove(person)">
{{person.name}}
<mat-icon matChipRemove>cancel</mat-icon>
</mat-chip-row>
</mat-chip-grid>
<input placeholder="New Contributor..."
[matChipInputFor]="chipGrid2"
<input [matChipInputFor]="chipGrid2"
[matChipInputSeparatorKeyCodes]="separatorKeysCodes"
[matChipInputAddOnBlur]="addOnBlur"
(matChipInputTokenEnd)="add($event)" />
Expand Down
2 changes: 1 addition & 1 deletion src/dev-app/mdc-chips/mdc-chips-demo.scss
Expand Up @@ -4,7 +4,7 @@
max-width: 200px;
}

.mat-card {
.mat-mdc-card {
padding: 0;
margin: 16px;

Expand Down
2 changes: 1 addition & 1 deletion src/dev-app/mdc-chips/mdc-chips-demo.ts
Expand Up @@ -8,7 +8,7 @@

import {COMMA, ENTER} from '@angular/cdk/keycodes';
import {Component} from '@angular/core';
import {ThemePalette} from '@angular/material/core';
import {ThemePalette} from '@angular/material-experimental/mdc-core';
import {MatChipInputEvent, MatChipEditedEvent} from '@angular/material-experimental/mdc-chips';

export interface Person {
Expand Down
10 changes: 7 additions & 3 deletions src/material-experimental/mdc-chips/BUILD.bazel
Expand Up @@ -22,7 +22,7 @@ ng_module(
deps = [
"//src:dev_mode_types",
"//src/material-experimental/mdc-core",
"//src/material/form-field",
"//src/material-experimental/mdc-form-field",
"@npm//@angular/animations",
"@npm//@angular/common",
"@npm//@angular/core",
Expand Down Expand Up @@ -69,8 +69,8 @@ ng_test_library(
"//src/cdk/testing",
"//src/cdk/testing/private",
"//src/material-experimental/mdc-core",
"//src/material/form-field",
"//src/material/input",
"//src/material-experimental/mdc-form-field",
"//src/material-experimental/mdc-input",
"@npm//@angular/animations",
"@npm//@angular/common",
"@npm//@angular/forms",
Expand All @@ -84,6 +84,10 @@ ng_test_library(
ng_web_test_suite(
name = "unit_tests",
static_files = [
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
"@npm//:node_modules/@material/chips/dist/mdc.chips.js",
"@npm//:node_modules/@material/ripple/dist/mdc.ripple.js",
],
Expand Down
20 changes: 10 additions & 10 deletions src/material-experimental/mdc-chips/chip-grid.spec.ts
Expand Up @@ -33,8 +33,8 @@ import {
} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {FormControl, FormsModule, NgForm, ReactiveFormsModule, Validators} from '@angular/forms';
import {MatFormFieldModule} from '@angular/material/form-field';
import {MatInputModule} from '@angular/material/input';
import {MatFormFieldModule} from '@angular/material-experimental/mdc-form-field';
import {MatInputModule} from '@angular/material-experimental/mdc-input';
import {By} from '@angular/platform-browser';
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
import {Subject} from 'rxjs';
Expand Down Expand Up @@ -668,7 +668,6 @@ describe('MDC-based MatChipGrid', () => {
.toEqual(null, `Expected the control's value to be empty initially.`);

const nativeInput = fixture.nativeElement.querySelector('input');
// tick();
nativeInput.focus();

typeInElement(nativeInput, '123');
Expand Down Expand Up @@ -750,20 +749,20 @@ describe('MDC-based MatChipGrid', () => {
});

it('should set an asterisk after the placeholder if the control is required', () => {
let requiredMarker = fixture.debugElement.query(By.css('.mat-form-field-required-marker'))!;
let requiredMarker = fixture.debugElement.query(By.css('.mdc-floating-label--required'))!;
expect(requiredMarker)
.toBeNull(`Expected placeholder not to have an asterisk, as control was not required.`);

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

requiredMarker = fixture.debugElement.query(By.css('.mat-form-field-required-marker'))!;
requiredMarker = fixture.debugElement.query(By.css('.mdc-floating-label--required'))!;
expect(requiredMarker)
.not.toBeNull(`Expected placeholder to have an asterisk, as control was required.`);
});

it('should blur the form field when the active chip is blurred', fakeAsync(() => {
const formField: HTMLElement = fixture.nativeElement.querySelector('.mat-form-field');
const formField: HTMLElement = fixture.nativeElement.querySelector('.mat-mdc-form-field');

dispatchFakeEvent(nativeChips[0], 'focusin');
fixture.detectChanges();
Expand Down Expand Up @@ -926,8 +925,8 @@ describe('MDC-based MatChipGrid', () => {
});

it('sets the aria-describedby to reference errors when in error state', () => {
let hintId =
fixture.debugElement.query(By.css('.mat-hint'))!.nativeElement.getAttribute('id');
let hintId = fixture.debugElement.query(By.css('.mat-mdc-form-field-hint'))!.nativeElement
.getAttribute('id');
let describedBy = chipGridEl.getAttribute('aria-describedby');

expect(hintId).toBeTruthy('hint should be shown');
Expand All @@ -936,7 +935,7 @@ describe('MDC-based MatChipGrid', () => {
fixture.componentInstance.formControl.markAsTouched();
fixture.detectChanges();

let errorIds = fixture.debugElement.queryAll(By.css('.mat-error'))
let errorIds = fixture.debugElement.queryAll(By.css('.mat-mdc-form-field-error'))
.map(el => el.nativeElement.getAttribute('id')).join(' ');
describedBy = chipGridEl.getAttribute('aria-describedby');

Expand Down Expand Up @@ -1039,13 +1038,14 @@ class FormFieldChipGrid {
@Component({
template: `
<mat-form-field>
<mat-label>New food...</mat-label>
<mat-chip-grid #chipGrid
placeholder="Food" [formControl]="control" [required]="isRequired">
<mat-chip-row *ngFor="let food of foods" [value]="food.value" (removed)="remove(food)">
{{ food.viewValue }}
</mat-chip-row>
</mat-chip-grid>
<input placeholder="New food..."
<input
[matChipInputFor]="chipGrid"
[matChipInputSeparatorKeyCodes]="separatorKeyCodes"
[matChipInputAddOnBlur]="addOnBlur"
Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-chips/chip-grid.ts
Expand Up @@ -34,7 +34,7 @@ import {
ErrorStateMatcher,
mixinErrorState,
} from '@angular/material-experimental/mdc-core';
import {MatFormFieldControl} from '@angular/material/form-field';
import {MatFormFieldControl} from '@angular/material-experimental/mdc-form-field';
import {MatChipTextControl} from './chip-text-control';
import {merge, Observable, Subscription} from 'rxjs';
import {startWith, takeUntil} from 'rxjs/operators';
Expand Down
27 changes: 11 additions & 16 deletions src/material-experimental/mdc-chips/chip-input.spec.ts
Expand Up @@ -4,7 +4,7 @@ import {PlatformModule} from '@angular/cdk/platform';
import {dispatchKeyboardEvent} from '@angular/cdk/testing/private';
import {Component, DebugElement, ViewChild} from '@angular/core';
import {waitForAsync, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {MatFormFieldModule} from '@angular/material/form-field';
import {MatFormFieldModule} from '@angular/material-experimental/mdc-form-field';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {Subject} from 'rxjs';
Expand All @@ -18,6 +18,10 @@ import {
} from './index';


// The following tests have been removed, because the use
// cases are not support by the MDC-based components:
// - should propagate the dynamic `placeholder` value to the form field

describe('MDC-based MatChipInput', () => {
let fixture: ComponentFixture<any>;
let testChipInput: TestChipInput;
Expand Down Expand Up @@ -74,21 +78,6 @@ describe('MDC-based MatChipInput', () => {
expect(inputNativeElement.getAttribute('placeholder')).toBe('bound placeholder');
});

it('should propagate the dynamic `placeholder` value to the form field', () => {
fixture.componentInstance.placeholder = 'add a chip';
fixture.detectChanges();

const label: HTMLElement = fixture.nativeElement.querySelector('.mat-form-field-label');

expect(label).toBeTruthy();
expect(label.textContent).toContain('add a chip');

fixture.componentInstance.placeholder = 'or don\'t';
fixture.detectChanges();

expect(label.textContent).toContain('or don\'t');
});

it('should become disabled if the list is disabled', () => {
expect(inputNativeElement.hasAttribute('disabled')).toBe(false);
expect(chipInputDirective.disabled).toBe(false);
Expand Down Expand Up @@ -143,6 +132,12 @@ describe('MDC-based MatChipInput', () => {
expect(gridElement.getAttribute('tabindex')).toBe('0', 'Expected tabindex to remain 0');
}));

it('should set input styling classes', () => {
expect(inputNativeElement.classList).toContain('mat-mdc-input-element');
expect(inputNativeElement.classList).toContain('mat-mdc-chip-input');
expect(inputNativeElement.classList).toContain('mdc-text-field__input');
});

});

describe('[addOnBlur]', () => {
Expand Down
5 changes: 4 additions & 1 deletion src/material-experimental/mdc-chips/chip-input.ts
Expand Up @@ -34,7 +34,10 @@ let nextUniqueId = 0;
selector: 'input[matChipInputFor]',
exportAs: 'matChipInput, matChipInputFor',
host: {
'class': 'mat-mdc-chip-input mat-input-element',
// TODO: eventually we should remove `mat-input-element` from here since it comes from the
// non-MDC version of the input. It's currently being kept for backwards compatibility, because
// the MDC chips were landed initially with it.
'class': 'mat-mdc-chip-input mat-mdc-input-element mdc-text-field__input mat-input-element',
'(keydown)': '_keydown($event)',
'(blur)': '_blur()',
'(focus)': '_focus()',
Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-chips/chips.scss
Expand Up @@ -108,7 +108,7 @@ $mat-mdc-chip-margin: 4px;
margin: -$mat-mdc-chip-margin;

// Keep the mat-chip-grid height the same even when there are no chips.
input.mat-input-element {
input.mat-mdc-chip-input {
margin: $mat-mdc-chip-margin;
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/material-experimental/mdc-chips/testing/BUILD.bazel
Expand Up @@ -29,6 +29,10 @@ ng_test_library(
ng_web_test_suite(
name = "unit_tests",
static_files = [
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
"@npm//:node_modules/@material/chips/dist/mdc.chips.js",
"@npm//:node_modules/@material/ripple/dist/mdc.ripple.js",
],
Expand Down
5 changes: 5 additions & 0 deletions src/material/chips/chip-input.spec.ts
Expand Up @@ -139,6 +139,11 @@ describe('MatChipInput', () => {
expect(inputNativeElement.getAttribute('aria-required')).toBe('true');
});

it('should set input styling classes', () => {
expect(inputNativeElement.classList).toContain('mat-input-element');
expect(inputNativeElement.classList).toContain('mat-chip-input');
});

});

describe('[addOnBlur]', () => {
Expand Down

0 comments on commit 7596532

Please sign in to comment.