Skip to content

Commit

Permalink
fix(select): consistent error behavior to md-input-container (#4754)
Browse files Browse the repository at this point in the history
* Gets `md-select` to behave in the same way as `md-input-container` when it comes to errors. This means highlighting itself when it is invalid and touched, or one of the parent forms/form groups is submitted.
* Moves the error state logic into a separate function in order to avoid some hard-to-follow selectors and to potentially allow overrides. This should also be a first step to supporting `md-error` inside `md-select`.
* Changes the required asterisk to always have the theme warn color, similarly to the input asterisk.

Fixes #4611.
  • Loading branch information
crisbeto authored and andrewseguin committed Jul 27, 2017
1 parent 0a5489f commit 6f73b35
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 11 deletions.
3 changes: 1 addition & 2 deletions src/lib/select/_select-theme.scss
Expand Up @@ -66,8 +66,7 @@
}
}

.mat-select:focus:not(.mat-select-disabled).mat-warn,
.mat-select:not(:focus).ng-invalid.ng-touched:not(.mat-select-disabled) {
.mat-select:focus:not(.mat-select-disabled).mat-warn, .mat-select-invalid {
@include _mat-select-inner-content-theme($warn);
}
}
Expand Down
115 changes: 107 additions & 8 deletions src/lib/select/select.spec.ts
@@ -1,5 +1,3 @@
import {TestBed, async, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {
Component,
DebugElement,
Expand All @@ -9,17 +7,26 @@ import {
ChangeDetectionStrategy,
OnInit,
} from '@angular/core';
import {
ControlValueAccessor,
FormControl,
FormsModule,
NG_VALUE_ACCESSOR,
ReactiveFormsModule,
FormGroup,
FormGroupDirective,
Validators,
} from '@angular/forms';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {TestBed, async, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing';
import {MdSelectModule} from './index';
import {OverlayContainer} from '../core/overlay/overlay-container';
import {MdSelect} from './select';
import {getMdSelectDynamicMultipleError, getMdSelectNonArrayValueError} from './select-errors';
import {MdOption} from '../core/option/option';
import {Directionality} from '../core/bidi/index';
import {DOWN_ARROW, UP_ARROW, ENTER, SPACE, HOME, END, TAB} from '../core/keyboard/keycodes';
import {
ControlValueAccessor, FormControl, FormsModule, NG_VALUE_ACCESSOR, ReactiveFormsModule
} from '@angular/forms';
import {Subject} from 'rxjs/Subject';
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
import {dispatchFakeEvent, dispatchKeyboardEvent, wrappedErrorMessage} from '@angular/cdk/testing';
Expand Down Expand Up @@ -66,7 +73,8 @@ describe('MdSelect', () => {
InvalidSelectInForm,
BasicSelectWithoutForms,
BasicSelectWithoutFormsPreselected,
BasicSelectWithoutFormsMultiple
BasicSelectWithoutFormsMultiple,
SelectInsideFormGroup
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -1719,11 +1727,12 @@ describe('MdSelect', () => {
'mat-select-required', `Expected the mat-select-required class to be set.`);
});

it('should set aria-invalid for selects that are invalid', () => {
it('should set aria-invalid for selects that are invalid and touched', () => {
expect(select.getAttribute('aria-invalid'))
.toEqual('false', `Expected aria-invalid attr to be false for valid selects.`);

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

expect(select.getAttribute('aria-invalid'))
Expand Down Expand Up @@ -2571,6 +2580,77 @@ describe('MdSelect', () => {

});

describe('error state', () => {
let fixture: ComponentFixture<SelectInsideFormGroup>;
let testComponent: SelectInsideFormGroup;
let select: HTMLElement;

beforeEach(() => {
fixture = TestBed.createComponent(SelectInsideFormGroup);
fixture.detectChanges();
testComponent = fixture.componentInstance;
select = fixture.debugElement.query(By.css('md-select')).nativeElement;
});

it('should not set the invalid class on a clean select', () => {
expect(testComponent.formGroup.untouched).toBe(true, 'Expected the form to be untouched.');
expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid.');
expect(select.classList)
.not.toContain('mat-select-invalid', 'Expected select not to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to false.');
});

it('should appear as invalid if it becomes touched', () => {
expect(select.classList)
.not.toContain('mat-select-invalid', 'Expected select not to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to false.');

testComponent.formControl.markAsTouched();
fixture.detectChanges();

expect(select.classList)
.toContain('mat-select-invalid', 'Expected select to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('true', 'Expected aria-invalid to be set to true.');
});

it('should not have the invalid class when the select becomes valid', () => {
testComponent.formControl.markAsTouched();
fixture.detectChanges();

expect(select.classList)
.toContain('mat-select-invalid', 'Expected select to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('true', 'Expected aria-invalid to be set to true.');

testComponent.formControl.setValue('pizza-1');
fixture.detectChanges();

expect(select.classList)
.not.toContain('mat-select-invalid', 'Expected select not to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to false.');
});

it('should appear as invalid when the parent form group is submitted', () => {
expect(select.classList)
.not.toContain('mat-select-invalid', 'Expected select not to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to false.');

dispatchFakeEvent(fixture.debugElement.query(By.css('form')).nativeElement, 'submit');
fixture.detectChanges();

expect(select.classList)
.toContain('mat-select-invalid', 'Expected select to appear invalid.');
expect(select.getAttribute('aria-invalid'))
.toBe('true', 'Expected aria-invalid to be set to true.');
});

});

});


Expand Down Expand Up @@ -2918,6 +2998,7 @@ class BasicSelectWithTheming {
theme: string;
}


@Component({
selector: 'reset-values-select',
template: `
Expand All @@ -2944,7 +3025,6 @@ class ResetValuesSelect {
@ViewChild(MdSelect) select: MdSelect;
}


@Component({
template: `
<md-select [formControl]="control">
Expand Down Expand Up @@ -3028,6 +3108,25 @@ class InvalidSelectInForm {
}


@Component({
template: `
<form [formGroup]="formGroup">
<md-select placeholder="Food" formControlName="food">
<md-option value="steak-0">Steak</md-option>
<md-option value="pizza-1">Pizza</md-option>
</md-select>
</form>
`
})
class SelectInsideFormGroup {
@ViewChild(FormGroupDirective) formGroupDirective: FormGroupDirective;
formControl = new FormControl('', Validators.required);
formGroup = new FormGroup({
food: this.formControl
});
}


@Component({
template: `
<md-select placeholder="Food" [(value)]="selectedFood">
Expand Down
17 changes: 16 additions & 1 deletion src/lib/select/select.ts
Expand Up @@ -28,6 +28,7 @@ import {
ChangeDetectionStrategy,
InjectionToken,
} from '@angular/core';
import {NgForm, FormGroupDirective} from '@angular/forms';
import {MdOption, MdOptionSelectionChange, MdOptgroup} from '../core/option/index';
import {ENTER, SPACE, UP_ARROW, DOWN_ARROW, HOME, END} from '../core/keyboard/keycodes';
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
Expand Down Expand Up @@ -153,9 +154,10 @@ export const _MdSelectMixinBase = mixinColor(mixinDisabled(MdSelectBase), 'prima
'[attr.aria-labelledby]': 'ariaLabelledby',
'[attr.aria-required]': 'required.toString()',
'[attr.aria-disabled]': 'disabled.toString()',
'[attr.aria-invalid]': '_control?.invalid || "false"',
'[attr.aria-invalid]': '_isErrorState()',
'[attr.aria-owns]': '_optionIds',
'[class.mat-select-disabled]': 'disabled',
'[class.mat-select-invalid]': '_isErrorState()',
'[class.mat-select-required]': 'required',
'class': 'mat-select',
'(keydown)': '_handleClosedKeydown($event)',
Expand Down Expand Up @@ -368,10 +370,13 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
renderer: Renderer2,
elementRef: ElementRef,
@Optional() private _dir: Directionality,
@Optional() private _parentForm: NgForm,
@Optional() private _parentFormGroup: FormGroupDirective,
@Self() @Optional() public _control: NgControl,
@Attribute('tabindex') tabIndex: string,
@Optional() @Inject(MD_PLACEHOLDER_GLOBAL_OPTIONS) placeholderOptions: PlaceholderOptions,
@Inject(MD_SELECT_SCROLL_STRATEGY) private _scrollStrategyFactory) {

super(renderer, elementRef);

if (this._control) {
Expand Down Expand Up @@ -605,6 +610,16 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
return this._selectionModel && this._selectionModel.hasValue();
}

/** Whether the select is in an error state. */
_isErrorState(): boolean {
const isInvalid = this._control && this._control.invalid;
const isTouched = this._control && this._control.touched;
const isSubmitted = (this._parentFormGroup && this._parentFormGroup.submitted) ||
(this._parentForm && this._parentForm.submitted);

return !!(isInvalid && (isTouched || isSubmitted));
}

/**
* Sets the scroll position of the scroll container. This must be called after
* the overlay pane is attached or the scroll container element will not yet be
Expand Down

0 comments on commit 6f73b35

Please sign in to comment.