-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(select): unable to programmatically select falsy values #4868
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but some nits. Apply merge label when ready.
src/lib/select/select.spec.ts
Outdated
@@ -449,6 +450,22 @@ describe('MdSelect', () => { | |||
expect(fixture.componentInstance.select.selected).not.toBeDefined(); | |||
}); | |||
|
|||
it('should be able to programmatically select a falsy option', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Why is this in its own section? Seems like it would better fit under forms integration since it has to do with setting the form control value.
src/lib/select/select.spec.ts
Outdated
it('should be able to programmatically select a falsy option', () => { | ||
fixture.destroy(); | ||
|
||
let falsyFixture = TestBed.createComponent(FalsyValueSelect); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: const
src/lib/select/select.spec.ts
Outdated
let falsyFixture = TestBed.createComponent(FalsyValueSelect); | ||
|
||
falsyFixture.detectChanges(); | ||
falsyFixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clicking on the trigger doesn't seem necessary if you're trying to test setting the value programmatically. Remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's there since we're also doing an extra check that the selected option has the proper class.
Fixes not being able to set falsy values progammatically in `md-select`. Fixes angular#4854.
99ecc03
to
d734c60
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes not being able to set falsy values progammatically in
md-select
.Fixes #4854.