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
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 committed Jun 28, 2017
1 parent f73cc97 commit 4635ac1
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
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<div class="mat-select-trigger" cdk-overlay-origin (click)="toggle()" #origin="cdkOverlayOrigin" #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
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ describe('MdSelect', () => {
BasicSelectWithTheming,
ResetValuesSelect,
FalsyValueSelect,
SelectWithGroups
SelectWithGroups,
InvalidSelectInForm
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -1942,6 +1943,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()).toThrow();

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

});

describe('change event', () => {
Expand Down Expand Up @@ -2842,3 +2854,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
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,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 @@ -771,7 +776,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 4635ac1

Please sign in to comment.