Skip to content

Commit

Permalink
fix(buttons): make buttons selectors more specific
Browse files Browse the repository at this point in the history 
BREAKING CHANGE:

Selectors for radio buttons related directives were changed and now both label
and input require ng-bootstrap specific attributes as selectors.

Before:

```
<div [(ngModel)]="model" ngbRadioGroup>
  <label class="btn">
    <input type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
  </label>
  <label class="btn">
    <input type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
  </label>
</div>
```

After:

```
<div [(ngModel)]="model" ngbRadioGroup>
  <label ngbButtonLabel>
    <input ngbButton type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
  </label>
  <label ngbButtonLabel>
    <input ngbButton type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
  </label>
</div>
```

Notice new `ngbButtonLabel` and `ngbButton` attributes that act as new selectors.

Fixes #1125

Closes #1678
  • Loading branch information
@pkozlowski-opensource
pkozlowski-opensource committed Jul 17, 2017
1 parent 55a91cb commit f9bc76e
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 194 deletions.
12 changes: 6 additions & 6 deletions demo/src/app/components/buttons/demos/radio-reactive/buttons-radio-reactive.html
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<form [formGroup]="radioGroupForm">
<div ngbRadioGroup name="radioBasic" formControlName="model">
<label class="btn btn-primary">
<input type="radio" [value]="1"> Left (pre-checked)
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" [value]="1"> Left (pre-checked)
</label>
<label class="btn btn-primary">
<input type="radio" value="middle"> Middle
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" value="middle"> Middle
</label>
<label class="btn btn-primary">
<input type="radio" [value]="false"> Right
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" [value]="false"> Right
</label>
</div>
</form>
Expand Down
12 changes: 6 additions & 6 deletions demo/src/app/components/buttons/demos/radio/buttons-radio.html
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<div [(ngModel)]="model" ngbRadioGroup name="radioBasic">
<label class="btn btn-primary">
<input type="radio" [value]="1"> Left (pre-checked)
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" [value]="1"> Left (pre-checked)
</label>
<label class="btn btn-primary">
<input type="radio" value="middle"> Middle
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" value="middle"> Middle
</label>
<label class="btn btn-primary">
<input type="radio" [value]="false"> Right
<label ngbButtonLabel class="btn-primary">
<input ngbButton type="radio" [value]="false"> Right
</label>
</div>
<hr>
Expand Down
6 changes: 3 additions & 3 deletions src/buttons/radio.module.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {NgModule, ModuleWithProviders} from '@angular/core';
import {NgbRadio, NgbActiveLabel, NgbRadioGroup} from './radio';
import {NgbRadio, NgbButtonLabel, NgbRadioGroup} from './radio';

export {NgbRadio, NgbActiveLabel, NgbRadioGroup} from './radio';
export {NgbRadio, NgbButtonLabel, NgbRadioGroup} from './radio';

const NGB_RADIO_DIRECTIVES = [NgbRadio, NgbActiveLabel, NgbRadioGroup];
const NGB_RADIO_DIRECTIVES = [NgbRadio, NgbButtonLabel, NgbRadioGroup];

@NgModule({declarations: NGB_RADIO_DIRECTIVES, exports: NGB_RADIO_DIRECTIVES})
export class NgbButtonsModule {
Expand Down
166 changes: 28 additions & 138 deletions src/buttons/radio.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,13 @@ function getLabel(nativeEl: HTMLElement, idx: number): HTMLElement {
return <HTMLElement>nativeEl.querySelectorAll('label')[idx];
}

describe('NgbActiveLabel', () => {
beforeEach(() => {
TestBed.configureTestingModule(
{declarations: [TestComponent], imports: [NgbButtonsModule, FormsModule, ReactiveFormsModule]});
});

it('should not touch active class on labels not part of a group', () => {
const fixture = createTestComponent('<label class="btn" [class.active]="true"></label>');
expect(fixture.nativeElement.children[0]).toHaveCssClass('active');
});
});

describe('ngbRadioGroup', () => {
const defaultHtml = `<div [(ngModel)]="model" ngbRadioGroup>
<label class="btn">
<input type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
<label ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label class="btn">
<input type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
<label ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>`;

Expand Down Expand Up @@ -199,8 +187,8 @@ describe('ngbRadioGroup', () => {
it('can be used with ngFor', async(() => {

const forHtml = `<div [(ngModel)]="model" ngbRadioGroup>
<label *ngFor="let v of values" class="btn">
<input type="radio" name="radio" [value]="v"/> {{ v }}
<label *ngFor="let v of values" ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="v"/> {{ v }}
</label>
</div>`;

Expand All @@ -220,11 +208,11 @@ describe('ngbRadioGroup', () => {
it('cleans up the model when radio inputs are added / removed', async(() => {

const ifHtml = `<div [(ngModel)]="model" ngbRadioGroup>
<label class="btn">
<input type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
<label ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label *ngIf="shown" class="btn">
<input type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
<label *ngIf="shown" ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>`;
const fixture = createTestComponent(ifHtml);
Expand Down Expand Up @@ -277,8 +265,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form>
<div ngbRadioGroup [(ngModel)]="model" name="control" required>
<label class="btn">
<input type="radio" value="foo"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo"/>
</label>
</div>
</form>`;
Expand All @@ -301,8 +289,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form [formGroup]="form">
<div ngbRadioGroup formControlName="control">
<label class="btn">
<input type="radio" value="foo"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo"/>
</label>
</div>
</form>`;
Expand All @@ -322,8 +310,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form [formGroup]="disabledForm">
<div ngbRadioGroup formControlName="control">
<label class="btn">
<input type="radio" value="foo"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo"/>
</label>
</div>
</form>`;
Expand All @@ -343,8 +331,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form>
<div ngbRadioGroup [(ngModel)]="model" name="control" [disabled]="disabled">
<label class="btn">
<input type="radio" value="foo"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo"/>
</label>
</div>
</form>`;
Expand Down Expand Up @@ -372,8 +360,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form>
<div ngbRadioGroup [(ngModel)]="model" name="control">
<label class="btn">
<input type="radio" value="foo" [disabled]="disabled"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo" [disabled]="disabled"/>
</label>
</div>
</form>`;
Expand Down Expand Up @@ -402,8 +390,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form>
<div ngbRadioGroup [(ngModel)]="model" name="control" [disabled]="groupDisabled">
<label class="btn">
<input type="radio" value="foo" [disabled]="disabled"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo" [disabled]="disabled"/>
</label>
</div>
</form>`;
Expand All @@ -430,8 +418,8 @@ describe('ngbRadioGroup', () => {
const html = `
<form>
<div ngbRadioGroup [(ngModel)]="model" name="control" [disabled]="groupDisabled">
<label class="btn">
<input type="radio" value="foo" [disabled]="disabled"/>
<label ngbButtonLabel>
<input ngbButton type="radio" value="foo" [disabled]="disabled"/>
</label>
</div>
</form>`;
Expand All @@ -454,106 +442,15 @@ describe('ngbRadioGroup', () => {
});
}));

it('should select a radio button when checked attribute is used', () => {
const html1 = `
<input type="radio" name="State" value="0" /> Foo <br>
<input type="radio" name="State" [value]="1" checked /> Foo Foo <br>`;

const fixture = createTestComponent(html1);
// checking initial values
fixture.detectChanges();

expect(getInput(fixture.nativeElement, 0).checked).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).checked).toBeTruthy();
});

it('should select a radio button when checked attribute with value is used', () => {
const html1 = `
<input type="radio" name="State" value="0" checked="checked"/> Foo <br>
<input type="radio" name="State" [value]="1" /> Foo Foo <br>`;

const fixture = createTestComponent(html1);
// checking initial values
fixture.detectChanges();

expect(getInput(fixture.nativeElement, 0).checked).toBeTruthy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

});

it('should disable a radio button when disabled attribute is used', () => {
const html1 = `
<input type="radio" name="State" value="0" /> Foo <br>
<input type="radio" name="State" [value]="1" disabled /> Foo Foo <br>`;

const fixture = createTestComponent(html1);
// checking initial values
fixture.detectChanges();

expect(getInput(fixture.nativeElement, 0).disabled).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).disabled).toBeTruthy();
});

it('should disable a radio button when disabled attribute with value is used', () => {
const html1 = `
<input type="radio" name="State" value="0" disabled="disabled"/> Foo <br>
<input type="radio" name="State" [value]="1" /> Foo Foo <br>`;

const fixture = createTestComponent(html1);
// checking initial values
fixture.detectChanges();

expect(getInput(fixture.nativeElement, 0).disabled).toBeTruthy();
expect(getInput(fixture.nativeElement, 1).disabled).toBeFalsy();
});

it('handle multiple cases for binded checked attribute.', () => {
const html1 = `
<input type="radio" name="State" value="0" [checked]="checked"/> Foo <br>
<input type="radio" name="State" [value]="1" /> Foo Foo <br>`;

const fixture = createTestComponent(html1);

// checking initial values which is undefined
fixture.detectChanges();
expect(getInput(fixture.nativeElement, 0).checked).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

// put checked to false
fixture.componentInstance.checked = false;
fixture.detectChanges();
expect(getInput(fixture.nativeElement, 0).checked).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

// put checked to null
fixture.componentInstance.checked = null;
fixture.detectChanges();
expect(getInput(fixture.nativeElement, 0).checked).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

// put checked to empty string
fixture.componentInstance.checked = '';
fixture.detectChanges();
expect(getInput(fixture.nativeElement, 0).checked).toBeFalsy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

// put checked to a string value
fixture.componentInstance.checked = 'false';
fixture.detectChanges();
// it should be true, anything else than "" is true
expect(getInput(fixture.nativeElement, 0).checked).toBeTruthy();
expect(getInput(fixture.nativeElement, 1).checked).toBeFalsy();

});

it('should add / remove "focus" class on labels', () => {
const fixture = createTestComponent(`
<div [(ngModel)]="model" ngbRadioGroup>
<label class="btn">
<input type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
<label ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label class="btn">
<input type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
<label ngbButtonLabel>
<input ngbButton type="radio" name="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>
`);
Expand All @@ -571,13 +468,6 @@ describe('ngbRadioGroup', () => {
expect(inputDebugEls[1].nativeElement.parentNode).toHaveCssClass('focus');
});

it('should do nothing when a standalone radio button is focused', () => {
const fixture = createTestComponent(`<input type="radio" name="state" value="0"/> Foo <br>`);
fixture.detectChanges();

expect(() => { fixture.debugElement.query(By.css('Input')).triggerEventHandler('focus', {}); }).not.toThrow();
});

describe('accessibility', () => {
it('should have "group" role', () => {
const fixture = TestBed.createComponent(TestComponent);
Expand Down

0 comments on commit f9bc76e

Please sign in to comment.