From 7463ead48ee0c169cd6b6ac9eea5a0e4863124ce Mon Sep 17 00:00:00 2001 From: "BLACKBAUD\\Trevor.Burch" Date: Thu, 14 Mar 2019 14:37:17 -0400 Subject: [PATCH 1/2] Added watches for when radio buttons change and the ability to set an aria label --- .../radio-group.component.fixture.html | 3 +- .../fixtures/radio-group.component.fixture.ts | 14 ++++ .../modules/radio/radio-group.component.html | 1 + .../radio/radio-group.component.spec.ts | 82 +++++++++++++++++++ .../modules/radio/radio-group.component.ts | 17 +++- 5 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/app/public/modules/radio/fixtures/radio-group.component.fixture.html b/src/app/public/modules/radio/fixtures/radio-group.component.fixture.html index 87530825..50d84031 100644 --- a/src/app/public/modules/radio/fixtures/radio-group.component.fixture.html +++ b/src/app/public/modules/radio/fixtures/radio-group.component.fixture.html @@ -1,9 +1,10 @@
diff --git a/src/app/public/modules/radio/radio-group.component.spec.ts b/src/app/public/modules/radio/radio-group.component.spec.ts index 76b322a4..1772fead 100644 --- a/src/app/public/modules/radio/radio-group.component.spec.ts +++ b/src/app/public/modules/radio/radio-group.component.spec.ts @@ -105,6 +105,46 @@ describe('Radio group component', function () { } })); + it('should maintain tabIndex when options change', fakeAsync(function () { + componentInstance.tabIndex = 2; + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + + componentInstance.changeOptions(); + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + + const radios = fixture.nativeElement.querySelectorAll('input'); + for (let element of radios) { + expect(element.getAttribute('tabindex')).toBe('2'); + } + })); + + it('should set the radio name properties correctly', fakeAsync(() => { + fixture.detectChanges(); + tick(); + const radios = fixture.nativeElement.querySelectorAll('input'); + for (let element of radios) { + expect(element.getAttribute('name')).toBe('option'); + } + })); + + it('should set the radio name properties correctly when options change', fakeAsync(() => { + fixture.detectChanges(); + tick(); + + componentInstance.changeOptions(); + fixture.detectChanges(); + tick(); + + const radios = fixture.nativeElement.querySelectorAll('input'); + for (let element of radios) { + expect(element.getAttribute('name')).toBe('option'); + } + })); + it('should maintain checked state when value is changed', fakeAsync(function () { fixture.detectChanges(); @@ -133,6 +173,48 @@ describe('Radio group component', function () { expect(radioDebugElement.componentInstance.checked).toBeTruthy(); })); + it('should maintain checked state when options are changed', fakeAsync(function () { + fixture.detectChanges(); + + let newValue = { + name: 'Jerry Salmonella', + disabled: false + }; + + let radioDebugElement = fixture.debugElement.query(By.css('sky-radio')); + radioDebugElement.componentInstance.value = newValue; + fixture.detectChanges(); + tick(); + + expect(radioDebugElement.componentInstance.checked).toBeTruthy(); + + componentInstance.changeOptions(); + + fixture.detectChanges(); + tick(); + + expect(radioDebugElement.componentInstance.checked).toBeTruthy(); + })); + + it('should set the aria-labeledby property correctly', fakeAsync(() => { + fixture.detectChanges(); + tick(); + + const radioGroupDiv = fixture.nativeElement.querySelector('.sky-radio-group'); + expect(radioGroupDiv.getAttribute('aria-labelledby')).toBe('radio-group-label'); + })); + + it('should set the aria-label property correctly', fakeAsync(() => { + componentInstance.ariaLabel = 'radio-group-label-manual'; + componentInstance.ariaLabelledBy = undefined; + + fixture.detectChanges(); + tick(); + + const radioGroupDiv = fixture.nativeElement.querySelector('.sky-radio-group'); + expect(radioGroupDiv.getAttribute('aria-label')).toBe('radio-group-label-manual'); + })); + it('should pass accessibility', async(() => { fixture.detectChanges(); fixture.whenStable().then(() => { diff --git a/src/app/public/modules/radio/radio-group.component.ts b/src/app/public/modules/radio/radio-group.component.ts index 35a9803f..f1feaba0 100644 --- a/src/app/public/modules/radio/radio-group.component.ts +++ b/src/app/public/modules/radio/radio-group.component.ts @@ -42,6 +42,9 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc @Input() public ariaLabelledBy: string; + @Input() + public ariaLabel: string; + @Input() public set name(value: string) { this._name = value; @@ -87,9 +90,7 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc private _tabIndex: number; public ngAfterContentInit(): void { - this.updateCheckedRadioFromValue(); - this.updateRadioButtonNames(); - this.updateRadioButtonTabIndexes(); + this.resetRadioButtons(); // Watch for radio selections. this.radios.forEach((radio) => { @@ -97,6 +98,10 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc this.writeValue(change.value); }); }); + + this.radios.changes.subscribe(() => { + this.resetRadioButtons(); + }); } public writeValue(value: any): void { @@ -145,4 +150,10 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc } }); } + + private resetRadioButtons(): void { + this.updateCheckedRadioFromValue(); + this.updateRadioButtonNames(); + this.updateRadioButtonTabIndexes(); + } } From 4b652ab27e201266f1c575e5080d785a1e370d3a Mon Sep 17 00:00:00 2001 From: "BLACKBAUD\\Trevor.Burch" Date: Tue, 19 Mar 2019 10:35:39 -0400 Subject: [PATCH 2/2] Code review changes --- .../modules/radio/radio-group.component.ts | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/app/public/modules/radio/radio-group.component.ts b/src/app/public/modules/radio/radio-group.component.ts index f1feaba0..cd6153cf 100644 --- a/src/app/public/modules/radio/radio-group.component.ts +++ b/src/app/public/modules/radio/radio-group.component.ts @@ -5,6 +5,7 @@ import { ContentChildren, forwardRef, Input, + OnDestroy, QueryList } from '@angular/core'; @@ -13,6 +14,10 @@ import { NG_VALUE_ACCESSOR } from '@angular/forms'; +import { + Subject +} from 'rxjs/Subject'; + import { SkyRadioChange } from './types'; @@ -38,7 +43,7 @@ const SKY_RADIO_GROUP_CONTROL_VALUE_ACCESSOR: any = { SKY_RADIO_GROUP_CONTROL_VALUE_ACCESSOR ] }) -export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAccessor { +export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAccessor, OnDestroy { @Input() public ariaLabelledBy: string; @@ -85,6 +90,8 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc @ContentChildren(SkyRadioComponent, { descendants: true }) private radios: QueryList; + private ngUnsubscribe = new Subject(); + private _name = `sky-radio-group-${nextUniqueId++}`; private _value: any; private _tabIndex: number; @@ -94,14 +101,23 @@ export class SkyRadioGroupComponent implements AfterContentInit, ControlValueAcc // Watch for radio selections. this.radios.forEach((radio) => { - radio.change.subscribe((change: SkyRadioChange) => { - this.writeValue(change.value); - }); + radio.change + .takeUntil(this.ngUnsubscribe) + .subscribe((change: SkyRadioChange) => { + this.writeValue(change.value); + }); }); - this.radios.changes.subscribe(() => { - this.resetRadioButtons(); - }); + this.radios.changes + .takeUntil(this.ngUnsubscribe) + .subscribe(() => { + this.resetRadioButtons(); + }); + } + + public ngOnDestroy() { + this.ngUnsubscribe.next(); + this.ngUnsubscribe.complete(); } public writeValue(value: any): void {