Skip to content

Commit

Permalink
fix(select): throwing additional errors if ngModel fails to initialize (
Browse files Browse the repository at this point in the history
#5405)

If the user has an `md-select` with an `ngModel` that doesn't have a name inside a form, the forms module will throw an error, however Material will also start throwing errors, which may cause confusion. These changes add a null check so our errors don't get mixed up with the forms error.

Fixes #5402.
  • Loading branch information
crisbeto authored and mmalerba committed Jul 9, 2017
1 parent b7efe48 commit 372549c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/lib/select/select.html
Expand Up @@ -7,11 +7,11 @@
#trigger>
<span
class="mat-select-placeholder"
[class.mat-floating-placeholder]="_selectionModel.hasValue()"
[class.mat-floating-placeholder]="_hasValue()"
[@transformPlaceholder]="_getPlaceholderAnimationState()"
[style.opacity]="_getPlaceholderOpacity()"
[style.width.px]="_selectedValueWidth"> {{ placeholder }} </span>
<span class="mat-select-value" *ngIf="_selectionModel.hasValue()">
<span class="mat-select-value" *ngIf="_hasValue()">
<span class="mat-select-value-text">{{ triggerValue }}</span>
</span>

Expand Down
22 changes: 21 additions & 1 deletion src/lib/select/select.spec.ts
Expand Up @@ -62,7 +62,8 @@ describe('MdSelect', () => {
BasicSelectWithTheming,
ResetValuesSelect,
FalsyValueSelect,
SelectWithGroups
SelectWithGroups,
InvalidSelectInForm
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -1972,6 +1973,17 @@ describe('MdSelect', () => {
}).not.toThrow();
}));

it('should not throw selection model-related errors in addition to the errors from ngModel',
async(() => {
const fixture = TestBed.createComponent(InvalidSelectInForm);

// The first change detection run will throw the "ngModel is missing a name" error.
expect(() => fixture.detectChanges()).toThrowError(/the name attribute must be set/g);

// The second run shouldn't throw selection-model related errors.
expect(() => fixture.detectChanges()).not.toThrow();
}));

});

describe('change event', () => {
Expand Down Expand Up @@ -2872,3 +2884,11 @@ class SelectWithGroups {
@ViewChild(MdSelect) select: MdSelect;
@ViewChildren(MdOption) options: QueryList<MdOption>;
}


@Component({
template: `<form><md-select [(ngModel)]="value"></md-select></form>`
})
class InvalidSelectInForm {
value: any;
}
7 changes: 6 additions & 1 deletion src/lib/select/select.ts
Expand Up @@ -548,6 +548,11 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
this._setScrollTop();
}

/** Whether the select has a value. */
_hasValue(): boolean {
return this._selectionModel && this._selectionModel.hasValue();
}

/**
* 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 Expand Up @@ -772,7 +777,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
// The farthest the panel can be scrolled before it hits the bottom
const maxScroll = scrollContainerHeight - panelHeight;

if (this._selectionModel.hasValue()) {
if (this._hasValue()) {
let selectedOptionOffset = this._getOptionIndex(this._selectionModel.selected[0])!;

selectedOptionOffset += this._getLabelCountBeforeOption(selectedOptionOffset);
Expand Down

0 comments on commit 372549c

Please sign in to comment.