Skip to content

Commit

Permalink
Prevent unhandled exceptions when pressing arrow keys with no availab…
Browse files Browse the repository at this point in the history
…le options (#202)

* Created failing test - shows arrow key can throw

Using the arrow keys to navigate the list of options will throw
an unhandled exception if there are no options in the list.

* Do not throw unhandled exception from _handleKeyup

Fixed a bug where the _handleKeyup would throw an unhandled exception
if there were no options in the select.

* fix announced option label

Co-authored-by: Esteban Gehring <esteban.gehring@gmail.com>
  • Loading branch information
josephdecock and macjohnny committed Jan 6, 2020
1 parent 4138c75 commit 9d7284c
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 7 deletions.
50 changes: 49 additions & 1 deletion src/app/mat-select-search/mat-select-search.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { takeUntil } from 'rxjs/operators';

import { MatSelectSearchComponent } from './mat-select-search.component';
import { NgxMatSelectSearchModule } from './ngx-mat-select-search.module';
import { LiveAnnouncer } from '@angular/cdk/a11y';
import { DOWN_ARROW } from '@angular/cdk/keycodes';

/* tslint:disable:component-selector */

Expand Down Expand Up @@ -261,7 +263,14 @@ describe('MatSelectSearchComponent', () => {
MatSelectModule,
NgxMatSelectSearchModule
],
declarations: [MatSelectSearchTestComponent]
declarations: [MatSelectSearchTestComponent],
providers: [{
provide: LiveAnnouncer,
useValue: {
announce: jasmine.createSpy('announce')
}
}
]
})
.compileComponents();
}));
Expand Down Expand Up @@ -391,6 +400,45 @@ describe('MatSelectSearchComponent', () => {

});

it('should not announce active option if there are no options', (done) => {
const announcer = TestBed.get(LiveAnnouncer);
component.filteredBanks
.pipe(
take(1),
delay(1)
)
.subscribe(() => {
// when the filtered banks are initialized
fixture.detectChanges();

component.matSelect.open();
fixture.detectChanges();

component.matSelect.openedChange
.pipe(take(1))
.subscribe((opened) => {

// search for "something definitely not in the list"
component.matSelectSearch.onInputChange('something definitely not in the list');
fixture.detectChanges();

component.filteredBanks
.pipe(take(1))
.subscribe(() => {
fixture.detectChanges();

setTimeout(() => {
expect(component.matSelect.options.length).toBe(0);

component.matSelectSearch._handleKeyup(<KeyboardEvent>{ keyCode: DOWN_ARROW });
expect(announcer.announce).not.toHaveBeenCalled();
done();
});
});
});
});
});

describe('inside mat-option', () => {

it('should show a search field and focus it when opening the select', (done) => {
Expand Down
14 changes: 8 additions & 6 deletions src/app/mat-select-search/mat-select-search.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,15 @@ export class MatSelectSearchComponent implements OnInit, OnDestroy, AfterViewIni
if (event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW) {
const ariaActiveDescendantId = this.matSelect._getAriaActiveDescendant();
const index = this._options.toArray().findIndex(item => item.id === ariaActiveDescendantId);
const activeDescendant = this._options.toArray()[index];
this.liveAnnouncer.announce(
' ' + activeDescendant.viewValue
+ this.getAriaIndex(index)
+ this.indexAndLengthScreenReaderText
+ this.getAriaLength()
if (index !== -1) {
const activeDescendant = this._options.toArray()[index];
this.liveAnnouncer.announce(
activeDescendant.viewValue + ' '
+ this.getAriaIndex(index)
+ this.indexAndLengthScreenReaderText
+ this.getAriaLength()
);
}
}
}

Expand Down

0 comments on commit 9d7284c

Please sign in to comment.