-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(checkbox): do not set indeterminate
when set checked
programmatically
#4024
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
Conversation
src/lib/checkbox/checkbox.spec.ts
Outdated
fixture.detectChanges(); | ||
|
||
fixture.whenStable().then(() => { | ||
fixture.detectChanges(); |
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.
Are the detectChanges
calls necessary after whenStable
? Also you may be able to avoid the nesting and the whenStable
calls by wrapping the test with fakeAsync
, instead of async
.
5caa8ed
to
2beb453
Compare
2beb453
to
9861af9
Compare
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
@@ -141,6 +141,31 @@ describe('MdCheckbox', () => { | |||
|
|||
})); | |||
|
|||
it('should not set indeterminate to false when checked is set programmatically', async(() => { |
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.
You may be able to avoid using async
here as well.
if (this._indeterminate) { | ||
Promise.resolve().then(() => { | ||
this._indeterminate = false; | ||
this.indeterminateChange.emit(this._indeterminate); |
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.
Can go in a follow-up PR, but this should only emit when the value is different
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. |
indeterminate
when user manually click on the checkbox