From 5fd7db14e4b9e56d01a7f2af573502f468afbc8a Mon Sep 17 00:00:00 2001 From: Dmitry Nehaychik <4dmitr@gmail.com> Date: Thu, 6 Jun 2019 10:44:45 +0300 Subject: [PATCH] refactor(checkbox): deprecate `value`, add `checked`, don't emit of input change Closes #1478 --- .../components/checkbox/checkbox.component.ts | 31 ++++++++--- .../components/checkbox/checkbox.spec.ts | 52 ++++++++++++++----- .../checkbox/checkbox-showcase.component.html | 5 +- .../checkbox/checkbox-showcase.component.ts | 6 +++ 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/src/framework/theme/components/checkbox/checkbox.component.ts b/src/framework/theme/components/checkbox/checkbox.component.ts index 4857330265..523051c3f1 100644 --- a/src/framework/theme/components/checkbox/checkbox.component.ts +++ b/src/framework/theme/components/checkbox/checkbox.component.ts @@ -157,17 +157,31 @@ export class NbCheckboxComponent implements ControlValueAccessor { /** * Checkbox value + * @deprecated + * @breaking-change Remove @5.0.0 */ @Input() get value(): boolean { - return this._value; + return this.checked; } + + /** + * @deprecated + */ set value(value: boolean) { - this._value = value; - this.valueChange.emit(value); + console.warn('NbCheckbox: `value` is deprecated and will be removed in 5.0.0. Use `checked` instead.'); + this.checked = value; + } + + @Input() + get checked(): boolean { + return this._checked; + } + set checked(value: boolean) { + this._checked = value; this.onChange(value); } - private _value: boolean = false; + private _checked: boolean = false; /** * Controls input disabled state @@ -201,7 +215,11 @@ export class NbCheckboxComponent implements ControlValueAccessor { } private _indeterminate: boolean = false; - @Output() valueChange = new EventEmitter(); + /** + * Output when value is changed by a user + * @type EventEmitter + */ + @Output() valueChange = new EventEmitter(); @HostBinding('class.status-primary') get primary() { @@ -239,7 +257,7 @@ export class NbCheckboxComponent implements ControlValueAccessor { } writeValue(val: any) { - this._value = val; + this._checked = val; this.changeDetector.detectChanges(); } @@ -254,6 +272,7 @@ export class NbCheckboxComponent implements ControlValueAccessor { updateValueAndIndeterminate(event: Event): void { const input = (event.target as HTMLInputElement); this.value = input.checked; + this.valueChange.emit(this.value); this.indeterminate = input.indeterminate; } } diff --git a/src/framework/theme/components/checkbox/checkbox.spec.ts b/src/framework/theme/components/checkbox/checkbox.spec.ts index df2c33dac4..48da1af344 100644 --- a/src/framework/theme/components/checkbox/checkbox.spec.ts +++ b/src/framework/theme/components/checkbox/checkbox.spec.ts @@ -1,5 +1,4 @@ -import { take } from 'rxjs/operators'; -import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Component, DebugElement } from '@angular/core'; import { By } from '@angular/platform-browser'; import { ReactiveFormsModule, FormControl } from '@angular/forms'; @@ -41,18 +40,30 @@ describe('Component: NbCheckbox', () => { expect(checkboxInput.nativeElement.disabled).toBeFalsy(); }); - it('Setting value to true makes checkbox input checked', () => { + it('Setting deprecated value to true makes checkbox input checked', () => { checkbox.value = true; fixture.detectChanges(); expect(checkboxInput.nativeElement.checked).toBeTruthy(); }); - it('Setting value to false makes checkbox input unchecked', () => { + it('Setting deprecated value to false makes checkbox input unchecked', () => { checkbox.value = false; fixture.detectChanges(); expect(checkboxInput.nativeElement.checked).toBeFalsy(); }); + it('Setting checked to true makes checkbox input checked', () => { + checkbox.checked = true; + fixture.detectChanges(); + expect(checkboxInput.nativeElement.checked).toBeTruthy(); + }); + + it('Setting checked to false makes checkbox input unchecked', () => { + checkbox.checked = false; + fixture.detectChanges(); + expect(checkboxInput.nativeElement.checked).toBeFalsy(); + }); + it('Setting status to success apply corresponding class to host element', () => { checkbox.status = 'success'; fixture.detectChanges(); @@ -83,26 +94,41 @@ describe('Component: NbCheckbox', () => { expect(testContainerEl.classList.contains('status-info')).toBeTruthy(); }); - it('should emit change event when changed', fakeAsync(() => { + it('should not emit change event when input changed', () => { + + const spy = jasmine.createSpy('valueChange subscriber'); + checkbox.valueChange - .pipe(take(1)) - .subscribe((value: boolean) => expect(value).toEqual(true)); + .subscribe(spy); - checkbox.value = true; + checkbox.checked = true; fixture.detectChanges(); - tick(); - })); + expect(spy).toHaveBeenCalledTimes(0); + }); + + it('should emit change event when clicked', () => { + + const spy = jasmine.createSpy('valueChange subscriber'); + + checkbox.valueChange + .subscribe(spy); + + label.nativeElement.click(); + + fixture.detectChanges(); + expect(spy).toHaveBeenCalledTimes(1); + }); it('should change value to opposite when clicked', () => { label.nativeElement.click(); fixture.detectChanges(); - expect(checkbox.value).toEqual(true); + expect(checkbox.checked).toEqual(true); label.nativeElement.click(); fixture.detectChanges(); - expect(checkbox.value).toEqual(false); + expect(checkbox.checked).toEqual(false); }); it('should reset indeterminate state when clicked on unchecked checkbox', () => { @@ -117,7 +143,7 @@ describe('Component: NbCheckbox', () => { }); it('should reset indeterminate state when clicked on unchecked checkbox', () => { - checkbox.value = false; + checkbox.checked = false; checkbox.indeterminate = true; fixture.detectChanges(); diff --git a/src/playground/with-layout/checkbox/checkbox-showcase.component.html b/src/playground/with-layout/checkbox/checkbox-showcase.component.html index 5b59897ade..5588ba6ef2 100644 --- a/src/playground/with-layout/checkbox/checkbox-showcase.component.html +++ b/src/playground/with-layout/checkbox/checkbox-showcase.component.html @@ -1,5 +1,8 @@ - Toggle me + Toggle me + + + {{ checked }} diff --git a/src/playground/with-layout/checkbox/checkbox-showcase.component.ts b/src/playground/with-layout/checkbox/checkbox-showcase.component.ts index 100fb6f768..0d340cd5bb 100644 --- a/src/playground/with-layout/checkbox/checkbox-showcase.component.ts +++ b/src/playground/with-layout/checkbox/checkbox-showcase.component.ts @@ -6,4 +6,10 @@ import { Component } from '@angular/core'; }) export class CheckboxShowcaseComponent { + + checked = false; + + toggle(checked: boolean) { + this.checked = checked; + } }