Skip to content

Commit

Permalink
fix(autocomplete): highlighted option not reset when closed with esca…
Browse files Browse the repository at this point in the history
…pe or enter key (#6403)

Fixes the autocomplete active option not being reset when closing through the escape or enter keys. Previously the old active option would remain active upon reopening, however the user's focus would shift back to the top as soon as they press an arrow key.

Fixes #6258.
  • Loading branch information
crisbeto authored and andrewseguin committed Aug 15, 2017
1 parent ea17d3d commit bf59468
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 7 deletions.
8 changes: 5 additions & 3 deletions src/lib/autocomplete/autocomplete-trigger.ts
Expand Up @@ -292,18 +292,20 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {

_handleKeydown(event: KeyboardEvent): void {
if (event.keyCode === ESCAPE && this.panelOpen) {
this._resetActiveItem();
this.closePanel();
event.stopPropagation();
} else if (this.activeOption && event.keyCode === ENTER && this.panelOpen) {
this.activeOption._selectViaInteraction();
this._resetActiveItem();
event.preventDefault();
} else {
const prevActiveItem = this.autocomplete._keyManager.activeItem;
const isArrowKey = event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW;

this.autocomplete._keyManager.onKeydown(event);

if (isArrowKey) {
if (this.panelOpen) {
this.autocomplete._keyManager.onKeydown(event);
} else if (isArrowKey) {
this.openPanel();
}

Expand Down
78 changes: 74 additions & 4 deletions src/lib/autocomplete/autocomplete.spec.ts
Expand Up @@ -640,6 +640,13 @@ describe('MdAutocomplete', () => {
tick();
fixture.detectChanges();

expect(fixture.componentInstance.trigger.panelOpen)
.toBe(true, 'Expected first down press to open the pane.');

fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
fixture.detectChanges();

expect(fixture.componentInstance.trigger.activeOption)
.toBe(fixture.componentInstance.options.first, 'Expected first option to be active.');
expect(optionEls[0].classList).toContain('mat-active');
Expand All @@ -665,6 +672,13 @@ describe('MdAutocomplete', () => {
tick();
fixture.detectChanges();

expect(fixture.componentInstance.trigger.panelOpen)
.toBe(true, 'Expected first up press to open the pane.');

fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
tick();
fixture.detectChanges();

expect(fixture.componentInstance.trigger.activeOption)
.toBe(fixture.componentInstance.options.last, 'Expected last option to be active.');
expect(optionEls[10].classList).toContain('mat-active');
Expand All @@ -686,6 +700,8 @@ describe('MdAutocomplete', () => {
fixture.detectChanges();

typeInElement('o', input);
fixture.detectChanges();

fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
fixture.detectChanges();
Expand Down Expand Up @@ -812,7 +828,7 @@ describe('MdAutocomplete', () => {
expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to scroll.`);

// These down arrows will set the 6th option active, below the fold.
[1, 2, 3, 4, 5].forEach(() => {
[1, 2, 3, 4, 5, 6].forEach(() => {
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
});
Expand Down Expand Up @@ -848,7 +864,7 @@ describe('MdAutocomplete', () => {
expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to scroll.`);

// These down arrows will set the 6th option active, below the fold.
[1, 2, 3, 4, 5].forEach(() => {
[1, 2, 3, 4, 5, 6].forEach(() => {
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
});
Expand Down Expand Up @@ -879,12 +895,12 @@ describe('MdAutocomplete', () => {
expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to scroll.`);

// These down arrows will set the 7th option active, below the fold.
[1, 2, 3, 4, 5, 6].forEach(() => {
[1, 2, 3, 4, 5, 6, 7].forEach(() => {
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
});

// These up arrows will set the 2nd option active
// These up arrows will set the 2nd option as active
[5, 4, 3, 2, 1].forEach(() => {
fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
tick();
Expand Down Expand Up @@ -914,6 +930,60 @@ describe('MdAutocomplete', () => {
});
}));

it('should reset the active option when closing with the escape key', fakeAsync(() => {
const trigger = fixture.componentInstance.trigger;

trigger.openPanel();
fixture.detectChanges();
tick();

expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
expect(!!trigger.activeOption).toBe(false, 'Expected no active option.');

// Press the down arrow a few times.
[1, 2, 3].forEach(() => {
trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
fixture.detectChanges();
});

// Note that this casts to a boolean, in order to prevent Jasmine
// from crashing when trying to stringify the option if the test fails.
expect(!!trigger.activeOption).toBe(true, 'Expected to find an active option.');

trigger._handleKeydown(createKeyboardEvent('keydown', ESCAPE));
tick();

expect(!!trigger.activeOption).toBe(false, 'Expected no active options.');
}));

it('should reset the active option when closing by selecting with enter', fakeAsync(() => {
const trigger = fixture.componentInstance.trigger;

trigger.openPanel();
fixture.detectChanges();
tick();

expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
expect(!!trigger.activeOption).toBe(false, 'Expected no active option.');

// Press the down arrow a few times.
[1, 2, 3].forEach(() => {
trigger._handleKeydown(DOWN_ARROW_EVENT);
tick();
fixture.detectChanges();
});

// Note that this casts to a boolean, in order to prevent Jasmine
// from crashing when trying to stringify the option if the test fails.
expect(!!trigger.activeOption).toBe(true, 'Expected to find an active option.');

trigger._handleKeydown(ENTER_EVENT);
tick();

expect(!!trigger.activeOption).toBe(false, 'Expected no active options.');
}));

});

describe('aria', () => {
Expand Down

0 comments on commit bf59468

Please sign in to comment.