Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(checkbox): deprecate value, add checked, don't emit of i… #1585

Merged
merged 6 commits into from
Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions src/framework/theme/components/checkbox/checkbox.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
* @breaking-change Remove @5.0.0
*/
set value(value: boolean) {
this._value = value;
this.valueChange.emit(value);
this.onChange(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;
}
private _value: boolean = false;
private _checked: boolean = false;

/**
* Controls input disabled state
Expand Down Expand Up @@ -201,7 +215,26 @@ export class NbCheckboxComponent implements ControlValueAccessor {
}
private _indeterminate: boolean = false;

@Output() valueChange = new EventEmitter();
/**
* Output when checked state is changed by a user
* @deprecated
* @breaking-change Remove @5.0.0
* @type EventEmitter<boolean>
*/
@Output()
get valueChange(): EventEmitter<boolean> {
console.warn('NbCheckbox: `valueChange` is deprecated and will be removed in 5.0.0. Use `checkedChange` instead.');
return this.checkedChange;
}
set valueChange(valueChange: EventEmitter<boolean>) {
this.checkedChange = valueChange;
}

/**
* Output when checked state is changed by a user
* @type EventEmitter<boolean>
*/
@Output() checkedChange = new EventEmitter<boolean>();

@HostBinding('class.status-primary')
get primary() {
Expand Down Expand Up @@ -239,7 +272,7 @@ export class NbCheckboxComponent implements ControlValueAccessor {
}

writeValue(val: any) {
this._value = val;
this._checked = val;
this.changeDetector.detectChanges();
}

Expand All @@ -253,7 +286,9 @@ export class NbCheckboxComponent implements ControlValueAccessor {

updateValueAndIndeterminate(event: Event): void {
const input = (event.target as HTMLInputElement);
this.value = input.checked;
this.checked = input.checked;
this.checkedChange.emit(this.checked);
this.onChange(this.checked);
this.indeterminate = input.indeterminate;
}
}
77 changes: 64 additions & 13 deletions src/framework/theme/components/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -83,26 +94,66 @@ describe('Component: NbCheckbox', () => {
expect(testContainerEl.classList.contains('status-info')).toBeTruthy();
});

it('should emit change event when changed', fakeAsync(() => {
it('deprecated should not emit change event when input changed', () => {

const spy = jasmine.createSpy('valueChange subscriber');

checkbox.valueChange
.subscribe(spy);

checkbox.checked = true;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(0);
});

it('deprecated should emit change event when clicked', () => {

const spy = jasmine.createSpy('valueChange subscriber');

checkbox.valueChange
.pipe(take(1))
.subscribe((value: boolean) => expect(value).toEqual(true));
.subscribe(spy);

label.nativeElement.click();

fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(1);
});

it('should not emit change event when input changed', () => {

const spy = jasmine.createSpy('checkedChange subscriber');

checkbox.checkedChange
.subscribe(spy);

checkbox.checked = true;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(0);
});

it('should emit change event when clicked', () => {

const spy = jasmine.createSpy('checkedChange subscriber');

checkbox.checkedChange
.subscribe(spy);

label.nativeElement.click();

checkbox.value = true;
fixture.detectChanges();
tick();
}));
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', () => {
Expand All @@ -117,7 +168,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();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<nb-card>
<nb-card-body>
<nb-checkbox disabled>Disabled</nb-checkbox>
<nb-checkbox [value]="true" indeterminate disabled>Indeterminate disabled</nb-checkbox>
<nb-checkbox [value]="true" disabled>Checked disabled</nb-checkbox>
<nb-checkbox [checked]="true" indeterminate disabled>Indeterminate disabled</nb-checkbox>
<nb-checkbox [checked]="true" disabled>Checked disabled</nb-checkbox>
</nb-card-body>
</nb-card>
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<nb-card>
<nb-card-body>
<nb-checkbox>Toggle me</nb-checkbox>
<nb-checkbox (checkedChange)="toggle($event)">Toggle me</nb-checkbox>
</nb-card-body>
<nb-card-body>
<label>Checked:</label> {{ checked }}
</nb-card-body>
</nb-card>
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ import { Component } from '@angular/core';
})

export class CheckboxShowcaseComponent {

checked = false;

toggle(checked: boolean) {
this.checked = checked;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import { Component } from '@angular/core';
<nb-checkbox id="first"></nb-checkbox>
</div>
<div>
<nb-checkbox id="second" [value]="true">Checked</nb-checkbox>
<nb-checkbox id="second" [checked]="true">Checked</nb-checkbox>
</div>
<div>
<nb-checkbox id="disabled" [disabled]="true">Disabled</nb-checkbox>
</div>
<div>
<nb-checkbox [value]="true" [disabled]="true">Disabled, checked</nb-checkbox>
<nb-checkbox [checked]="true" [disabled]="true">Disabled, checked</nb-checkbox>
</div>
<div>
<nb-checkbox id="success" status="success">Success</nb-checkbox>
Expand Down