Skip to content

Commit 97a9bdc

Browse files
authored
fix(radio): Make MdRadioButton change detection strategy OnPush (#2526)
* Make MdRadioButton OnPush * . * Fixed tests * fixed group ripple disabled test * fix lint * fix test * sync and fix test * address comments * address comments
1 parent 57c45a1 commit 97a9bdc

File tree

2 files changed

+70
-11
lines changed

2 files changed

+70
-11
lines changed

src/lib/radio/radio.spec.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ describe('MdRadio', () => {
229229
});
230230

231231
it('should not show ripples on disabled radio buttons', () => {
232-
radioInstances[0].disabled = true;
232+
testComponent.isFirstDisabled = true;
233233
fixture.detectChanges();
234234

235235
dispatchFakeEvent(radioLabelElements[0], 'mousedown');
@@ -238,7 +238,7 @@ describe('MdRadio', () => {
238238
expect(radioNativeElements[0].querySelectorAll('.mat-ripple-element').length)
239239
.toBe(0, 'Expected a disabled radio button to not show ripples');
240240

241-
radioInstances[0].disabled = false;
241+
testComponent.isFirstDisabled = false;
242242
fixture.detectChanges();
243243

244244
dispatchFakeEvent(radioLabelElements[0], 'mousedown');
@@ -417,11 +417,13 @@ describe('MdRadio', () => {
417417

418418
it('should write to the radio button based on ngModel', fakeAsync(() => {
419419
testComponent.modelValue = 'chocolate';
420+
420421
fixture.detectChanges();
421422
tick();
422423
fixture.detectChanges();
423424

424425
expect(innerRadios[1].nativeElement.checked).toBe(true);
426+
expect(radioInstances[1].checked).toBe(true);
425427
}));
426428

427429
it('should update the ngModel value when selecting a radio button', () => {
@@ -551,7 +553,7 @@ describe('MdRadio', () => {
551553
it('should change aria-label attribute if property is changed at runtime', () => {
552554
expect(fruitRadioNativeInputs[0].getAttribute('aria-label')).toBe('Banana');
553555

554-
fruitRadioInstances[0].ariaLabel = 'Pineapple';
556+
testComponent.ariaLabel = 'Pineapple';
555557
fixture.detectChanges();
556558

557559
expect(fruitRadioNativeInputs[0].getAttribute('aria-label')).toBe('Pineapple');
@@ -568,7 +570,7 @@ describe('MdRadio', () => {
568570
it('should change aria-labelledby attribute if property is changed at runtime', () => {
569571
expect(fruitRadioNativeInputs[0].getAttribute('aria-labelledby')).toBe('xyz');
570572

571-
fruitRadioInstances[0].ariaLabelledby = 'uvw';
573+
testComponent.ariaLabelledby = 'uvw';
572574
fixture.detectChanges();
573575

574576
expect(fruitRadioNativeInputs[0].getAttribute('aria-labelledby')).toBe('uvw');
@@ -593,7 +595,8 @@ describe('MdRadio', () => {
593595
[labelPosition]="labelPos"
594596
[value]="groupValue"
595597
name="test-name">
596-
<md-radio-button value="fire" [disableRipple]="disableRipple">Charmander</md-radio-button>
598+
<md-radio-button value="fire" [disableRipple]="disableRipple"
599+
[disabled]="isFirstDisabled">Charmander</md-radio-button>
597600
<md-radio-button value="water" [disableRipple]="disableRipple">Squirtle</md-radio-button>
598601
<md-radio-button value="leaf" [disableRipple]="disableRipple">Bulbasaur</md-radio-button>
599602
</md-radio-group>
@@ -602,6 +605,7 @@ describe('MdRadio', () => {
602605
class RadiosInsideRadioGroup {
603606
labelPos: 'before' | 'after';
604607
isGroupDisabled: boolean = false;
608+
isFirstDisabled: boolean = false;
605609
groupValue: string = null;
606610
disableRipple: boolean = false;
607611
}
@@ -618,12 +622,18 @@ class RadiosInsideRadioGroup {
618622
<md-radio-button name="weather" value="cool">Autumn</md-radio-button>
619623
620624
<span id="xyz">Baby Banana</span>
621-
<md-radio-button name="fruit" value="banana" aria-label="Banana" aria-labelledby="xyz">
625+
<md-radio-button name="fruit"
626+
value="banana"
627+
[aria-label]="ariaLabel"
628+
[aria-labelledby]="ariaLabelledby">
622629
</md-radio-button>
623630
<md-radio-button name="fruit" value="raspberry">Raspberry</md-radio-button>
624631
`
625632
})
626-
class StandaloneRadioButtons { }
633+
class StandaloneRadioButtons {
634+
ariaLabel: string = 'Banana';
635+
ariaLabelledby: string = 'xyz';
636+
}
627637

628638

629639
@Component({

src/lib/radio/radio.ts

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import {
22
AfterContentInit,
3+
ChangeDetectionStrategy,
4+
ChangeDetectorRef,
35
Component,
46
ContentChildren,
57
Directive,
@@ -86,6 +88,12 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
8688
/** Whether the `value` has been set to its initial value. */
8789
private _isInitialized: boolean = false;
8890

91+
/** Whether the labels should appear after or before the radio-buttons. Defaults to 'after' */
92+
private _labelPosition: 'before' | 'after' = 'after';
93+
94+
/** Whether the radio group is disabled. */
95+
private _disabled: boolean = false;
96+
8997
/** The method to be called in order to update ngModel */
9098
_controlValueAccessorChangeFn: (value: any) => void = (value) => {};
9199

@@ -129,8 +137,17 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
129137
this.labelPosition = (v == 'start') ? 'after' : 'before';
130138
}
131139

140+
132141
/** Whether the labels should appear after or before the radio-buttons. Defaults to 'after' */
133-
@Input() labelPosition: 'before' | 'after' = 'after';
142+
@Input()
143+
get labelPosition() {
144+
return this._labelPosition;
145+
}
146+
147+
set labelPosition(v) {
148+
this._labelPosition = (v == 'before') ? 'before' : 'after';
149+
this._markRadiosForCheck();
150+
}
134151

135152
/** Value of the radio button. */
136153
@Input()
@@ -160,6 +177,18 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
160177
this._checkSelectedRadioButton();
161178
}
162179

180+
/** Whether the radio group is diabled */
181+
@Input()
182+
get disabled() { return this._disabled; }
183+
set disabled(value) {
184+
this._disabled = value;
185+
this._markRadiosForCheck();
186+
}
187+
188+
constructor(private _changeDetector: ChangeDetectorRef) {
189+
super();
190+
}
191+
163192
/**
164193
* Initialize properties once content children are available.
165194
* This allows us to propagate relevant attributes to associated buttons.
@@ -215,12 +244,19 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
215244
}
216245
}
217246

247+
_markRadiosForCheck() {
248+
if (this._radios) {
249+
this._radios.forEach(radio => radio._markForCheck());
250+
}
251+
}
252+
218253
/**
219254
* Sets the model value. Implemented as part of ControlValueAccessor.
220255
* @param value
221256
*/
222257
writeValue(value: any) {
223258
this.value = value;
259+
this._changeDetector.markForCheck();
224260
}
225261

226262
/**
@@ -247,6 +283,7 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
247283
*/
248284
setDisabledState(isDisabled: boolean) {
249285
this.disabled = isDisabled;
286+
this._changeDetector.markForCheck();
250287
}
251288
}
252289

@@ -264,7 +301,8 @@ export class MdRadioGroup extends _MdRadioGroupMixinBase
264301
'[class.mat-radio-checked]': 'checked',
265302
'[class.mat-radio-disabled]': 'disabled',
266303
'[attr.id]': 'id',
267-
}
304+
},
305+
changeDetection: ChangeDetectionStrategy.OnPush,
268306
})
269307
export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
270308

@@ -307,6 +345,7 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
307345
// Notify all radio buttons with the same name to un-check.
308346
this._radioDispatcher.notify(this.id, this.name);
309347
}
348+
this._changeDetector.markForCheck();
310349
}
311350
}
312351

@@ -328,7 +367,6 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
328367
this.radioGroup.selected = this;
329368
}
330369
}
331-
332370
}
333371
}
334372

@@ -364,7 +402,6 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
364402
get disabled(): boolean {
365403
return this._disabled || (this.radioGroup != null && this.radioGroup.disabled);
366404
}
367-
368405
set disabled(value: boolean) {
369406
this._disabled = coerceBooleanProperty(value);
370407
}
@@ -408,6 +445,7 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
408445
constructor(@Optional() radioGroup: MdRadioGroup,
409446
private _elementRef: ElementRef,
410447
private _renderer: Renderer2,
448+
private _changeDetector: ChangeDetectorRef,
411449
private _focusOriginMonitor: FocusOriginMonitor,
412450
private _radioDispatcher: UniqueSelectionDispatcher) {
413451
// Assertions. Ideally these should be stripped out by the compiler.
@@ -427,6 +465,17 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy {
427465
this._focusOriginMonitor.focusVia(this._inputElement.nativeElement, this._renderer, 'keyboard');
428466
}
429467

468+
/**
469+
* Marks the radio button as needing checking for change detection.
470+
* This method is exposed because the parent radio group will directly
471+
* update bound properties of the radio button.
472+
*/
473+
_markForCheck() {
474+
// When group value changes, the button will not be notified. Use `markForCheck` to explicit
475+
// update radio button's status
476+
this._changeDetector.markForCheck();
477+
}
478+
430479
ngOnInit() {
431480
if (this.radioGroup) {
432481
// If the radio is inside a radio group, determine if it should be checked

0 commit comments

Comments
 (0)