Skip to content

Commit 083fb0f

Browse files
authored
fix(buttons): display correct NgModel value inside OnPush component (#3000)
Fixes #2980
1 parent 0e9b291 commit 083fb0f

File tree

4 files changed

+68
-12
lines changed

4 files changed

+68
-12
lines changed

src/buttons/checkbox.spec.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import {TestBed, ComponentFixture, async, fakeAsync, tick} from '@angular/core/testing';
1+
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
22
import {By} from '@angular/platform-browser';
33
import {createGenericTestComponent} from '../test/common';
44

5-
import {Component} from '@angular/core';
5+
import {ChangeDetectionStrategy, Component} from '@angular/core';
66
import {FormsModule, ReactiveFormsModule} from '@angular/forms';
77

88
import {NgbButtonsModule} from './buttons.module';
@@ -11,6 +11,9 @@ import {NgbCheckBox} from './checkbox';
1111
const createTestComponent = (html: string) =>
1212
createGenericTestComponent(html, TestComponent) as ComponentFixture<TestComponent>;
1313

14+
const createOnPushTestComponent = (html: string) =>
15+
createGenericTestComponent(html, TestComponentOnPush) as ComponentFixture<TestComponentOnPush>;
16+
1417
function getLabel(nativeEl: HTMLElement): HTMLElement {
1518
return <HTMLElement>nativeEl.querySelector('label');
1619
}
@@ -22,8 +25,10 @@ function getInput(nativeEl: HTMLElement): HTMLInputElement {
2225
describe('NgbCheckBox', () => {
2326

2427
beforeEach(() => {
25-
TestBed.configureTestingModule(
26-
{declarations: [TestComponent], imports: [NgbButtonsModule, FormsModule, ReactiveFormsModule]});
28+
TestBed.configureTestingModule({
29+
declarations: [TestComponent, TestComponentOnPush],
30+
imports: [NgbButtonsModule, FormsModule, ReactiveFormsModule]
31+
});
2732
});
2833

2934
describe('bindings', () => {
@@ -151,10 +156,30 @@ describe('NgbCheckBox', () => {
151156

152157
});
153158

159+
describe('on push', () => {
160+
it('should set initial model value', fakeAsync(() => {
161+
const fixture = createOnPushTestComponent(`
162+
<label ngbButtonLabel>
163+
<input type="checkbox" ngbButton [ngModel]="true">
164+
</label>
165+
`);
166+
167+
fixture.detectChanges();
168+
tick();
169+
fixture.detectChanges();
170+
expect(getInput(fixture.debugElement.nativeElement).checked).toBeTruthy();
171+
expect(getLabel(fixture.debugElement.nativeElement)).toHaveCssClass('active');
172+
}));
173+
});
174+
154175
});
155176

156177
@Component({selector: 'test-cmp', template: ''})
157178
class TestComponent {
158179
disabled;
159180
model;
160181
}
182+
183+
@Component({selector: 'test-cmp-on-push', template: '', changeDetection: ChangeDetectionStrategy.OnPush})
184+
class TestComponentOnPush {
185+
}

src/buttons/checkbox.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Directive, forwardRef, Input} from '@angular/core';
1+
import {ChangeDetectorRef, Directive, forwardRef, Input} from '@angular/core';
22
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
33

44
import {NgbButtonLabel} from './label';
@@ -54,7 +54,7 @@ export class NgbCheckBox implements ControlValueAccessor {
5454
}
5555
}
5656

57-
constructor(private _label: NgbButtonLabel) {}
57+
constructor(private _label: NgbButtonLabel, private _cd: ChangeDetectorRef) {}
5858

5959
onInputChange($event) {
6060
const modelToPropagate = $event.target.checked ? this.valueChecked : this.valueUnChecked;
@@ -75,5 +75,8 @@ export class NgbCheckBox implements ControlValueAccessor {
7575
writeValue(value) {
7676
this.checked = value === this.valueChecked;
7777
this._label.active = this.checked;
78+
79+
// label won't be updated, if it is inside the OnPush component when [ngModel] changes
80+
this._cd.markForCheck();
7881
}
7982
}

src/buttons/radio.spec.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import {Component} from '@angular/core';
2-
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
1+
import {ChangeDetectionStrategy, Component} from '@angular/core';
2+
import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
33
import {FormControl, FormGroup, FormsModule, NgModel, ReactiveFormsModule, Validators} from '@angular/forms';
44
import {By} from '@angular/platform-browser';
55

@@ -54,9 +54,12 @@ describe('ngbRadioGroup', () => {
5454
</div>`;
5555

5656
beforeEach(() => {
57-
TestBed.configureTestingModule(
58-
{declarations: [TestComponent], imports: [NgbButtonsModule, FormsModule, ReactiveFormsModule]});
57+
TestBed.configureTestingModule({
58+
declarations: [TestComponent, TestComponentOnPush],
59+
imports: [NgbButtonsModule, FormsModule, ReactiveFormsModule]
60+
});
5961
TestBed.overrideComponent(TestComponent, {set: {template: defaultHtml}});
62+
TestBed.overrideComponent(TestComponentOnPush, {set: {template: defaultHtml}});
6063
});
6164

6265
it('toggles radio inputs based on model changes', async(() => {
@@ -577,6 +580,20 @@ describe('ngbRadioGroup', () => {
577580
expect(getGroupElement(fixture.nativeElement).getAttribute('role')).toBe('group');
578581
});
579582
});
583+
584+
describe('on push', () => {
585+
it('should set initial model value', fakeAsync(() => {
586+
const fixture = TestBed.createComponent(TestComponentOnPush);
587+
const {values} = fixture.componentInstance;
588+
589+
fixture.detectChanges();
590+
tick();
591+
fixture.detectChanges();
592+
expect(getInput(fixture.nativeElement, 0).value).toEqual(values[0]);
593+
expect(getInput(fixture.nativeElement, 1).value).toEqual(values[1]);
594+
expectRadios(fixture.nativeElement, [1, 0]);
595+
}));
596+
});
580597
});
581598

582599
@Component({selector: 'test-cmp', template: ''})
@@ -592,3 +609,9 @@ class TestComponent {
592609
groupDisabled = true;
593610
checked: any;
594611
}
612+
613+
@Component({selector: 'test-cmp-on-push', template: '', changeDetection: ChangeDetectionStrategy.OnPush})
614+
class TestComponentOnPush {
615+
model = 'one';
616+
values = ['one', 'two', 'three'];
617+
}

src/buttons/radio.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Directive, ElementRef, forwardRef, Input, OnDestroy, Renderer2} from '@angular/core';
1+
import {ChangeDetectorRef, Directive, ElementRef, forwardRef, Input, OnDestroy, Renderer2} from '@angular/core';
22
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
33

44
import {NgbButtonLabel} from './label';
@@ -127,7 +127,7 @@ export class NgbRadio implements OnDestroy {
127127

128128
constructor(
129129
private _group: NgbRadioGroup, private _label: NgbButtonLabel, private _renderer: Renderer2,
130-
private _element: ElementRef<HTMLInputElement>) {
130+
private _element: ElementRef<HTMLInputElement>, private _cd: ChangeDetectorRef) {
131131
this._group.register(this);
132132
this.updateDisabled();
133133
}
@@ -137,6 +137,11 @@ export class NgbRadio implements OnDestroy {
137137
onChange() { this._group.onRadioChange(this); }
138138

139139
updateValue(value) {
140+
// label won't be updated, if it is inside the OnPush component when [ngModel] changes
141+
if (this.value !== value) {
142+
this._cd.markForCheck();
143+
}
144+
140145
this._checked = this.value === value;
141146
this._label.active = this._checked;
142147
}

0 commit comments

Comments
 (0)