Skip to content

Conversation

stevenyxu
Copy link
Contributor

@stevenyxu stevenyxu commented May 1, 2018

This is related to #11056, which is a similar fix for MatRadioButton.

The MatCheckbox component instance may be accessed directly by a parent component using
ViewChild/ContentChild. Since MatCheckbox uses OnPush change detection setting the disabled property
directly currently doesn't cause change detection leading to a stale or broken state. Accessing it
through a template binding does correctly trigger change detection on MatCheckbox (there are already
test cases for this method). We need to add a manual call to markForCheck, or else MatCheckbox will
never get kicked if disabled is set directly on the component instance.

Related open Angular bug: angular/angular#20611

…etection

This is related to angular#11056, which is a similar fix for
MatRadioButton.

The MatCheckbox component instance may be accessed directly by a parent component using
ViewChild/ContentChild. Since MatCheckbox uses OnPush change detection setting the disabled property
directly currently doesn't cause change detection leading to a stale or broken state. Accessing it
through a template binding does correctly trigger change detection on MatCheckbox (there are already
test cases for this method). We need to add a manual call to markForCheck, or else MatCheckbox will
never get kicked if disabled is set directly on the component instance.

Related open Angular bug: angular/angular#20611
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 1, 2018
@tinayuangao tinayuangao self-assigned this May 2, 2018
Copy link
Contributor

@tinayuangao tinayuangao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tinayuangao tinayuangao added the action: merge The PR is ready for merge by the caretaker label May 2, 2018
@ngbot
Copy link

ngbot bot commented May 2, 2018

I see that you just added the pr: merge ready label, but the following checks are still failing:
    failure missing required label: "target: *"
    failure status "continuous-integration/travis-ci/pr" is failing
    pending 1 pending code review

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@tinayuangao tinayuangao added pr: lgtm target: patch This PR is targeted for the next patch release labels May 2, 2018
@jelbourn jelbourn removed the request for review from devversion May 8, 2018 16:57
@jelbourn jelbourn merged commit ce1b517 into angular:master May 8, 2018
jelbourn pushed a commit to jelbourn/components that referenced this pull request May 14, 2018
…etection (angular#11098)

This is related to angular#11056, which is a similar fix for
MatRadioButton.

The MatCheckbox component instance may be accessed directly by a parent component using
ViewChild/ContentChild. Since MatCheckbox uses OnPush change detection setting the disabled property
directly currently doesn't cause change detection leading to a stale or broken state. Accessing it
through a template binding does correctly trigger change detection on MatCheckbox (there are already
test cases for this method). We need to add a manual call to markForCheck, or else MatCheckbox will
never get kicked if disabled is set directly on the component instance.

Related open Angular bug: angular/angular#20611
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants