From bf59468b01da78be38e4697f47c777ccd5e2ac2f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 15 Aug 2017 23:50:03 +0200 Subject: [PATCH] fix(autocomplete): highlighted option not reset when closed with escape 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. --- src/lib/autocomplete/autocomplete-trigger.ts | 8 +- src/lib/autocomplete/autocomplete.spec.ts | 78 +++++++++++++++++++- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index 62c97005858..969febb99ce 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -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(); } diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts index 95cfeb1d328..a20d0ab3d9e 100644 --- a/src/lib/autocomplete/autocomplete.spec.ts +++ b/src/lib/autocomplete/autocomplete.spec.ts @@ -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'); @@ -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'); @@ -686,6 +700,8 @@ describe('MdAutocomplete', () => { fixture.detectChanges(); typeInElement('o', input); + fixture.detectChanges(); + fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT); tick(); fixture.detectChanges(); @@ -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(); }); @@ -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(); }); @@ -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(); @@ -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', () => {