fix(radio): update MatRadioButton disabled setter to trigger change detection #11056
Conversation
googlebot
added
the
cla: yes
| @@ -410,7 +410,11 @@ export class MatRadioButton extends _MatRadioButtonMixinBase | |||
| return this._disabled || (this.radioGroup !== null && this.radioGroup.disabled); | |||
| } | |||
| set disabled(value: boolean) { | |||
| this._disabled = coerceBooleanProperty(value); | |||
| const newDisabledState = coerceBooleanProperty(value); | |||
There was a problem hiding this comment.
set disabled should trigger change detection with @Input()
There was a problem hiding this comment.
This does not appears to be the case. Demo: https://stackblitz.com/edit/angular-material-feiydh?file=app%2Fapp.component.html.
@input appears to cause for the setter to trigger change detection when tripped during template binding, but it doesn't appear to rewrite the setter implementation to always call change detection, so direct accesses to the component instance don't trigger change detection. Sometimes (e.g., when using ng-content), direct instance access is unavoidable, and there needs to be some way to disable a radio button. If there's a better way that I missed, let me know.
Potential reference (unresolved issue in Angular): angular/angular#20611
stevenyxu
force-pushed
the
master
branch
2 times, most recently
from
02587e5 to
5c47d56
Compare
stevenyxu
changed the title
… change detection Fixes MatRadioButton.disabled setter to trigger change detection. MatRadioButton uses ChangeDetectionStrategy.OnPush, which works fine with disabled is bound in a template that uses <mat-radio-button>. However, if a parent directly accesses the MatRadioButton component instance (e.g., using ViewChild, as in the provided test case), there is otherwise no way for the parent to trigger change detection in the MatRadioButton due to the OnPush strategy. OnPush was added in 97a9bdc, and this same technique was added to some but not all setters. This commit also adds test coverage for directly setting disabled on MatRadioButton in general, which previously was only indirectly covered in the MatRadioGroup tests.
|
I didn't mean to close this. Got my branches mixed up. |
tinayuangao
added
pr: lgtm
action: merge
|
I see that you just added the 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
added
the
target: patch
|
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 MatRadioButton.disabled setter to trigger change detection. MatRadioButton uses
ChangeDetectionStrategy.OnPush, which works fine with disabled is bound in a template that uses
. However, if a parent directly accesses the MatRadioButton component instance
(e.g., using ViewChild, as in the provided test case), there is otherwise no way for the parent to
trigger change detection in the MatRadioButton due to the OnPush strategy.
OnPush was added in 97a9bdc, and this same technique was added to some but not all setters.
This commit also adds test coverage for directly setting disabled on MatRadioButton in general,
which previously was only indirectly covered in the MatRadioGroup tests.
Related open Angular bug: angular/angular#20611