Skip to content

Commit a4e2de7

Browse files
karatinayuangao
authored andcommitted
fix(autocomplete): fix down arrow use with ngIf (#3493)
1 parent 4d4a63e commit a4e2de7

File tree

5 files changed

+186
-132
lines changed

5 files changed

+186
-132
lines changed

src/lib/autocomplete/autocomplete-trigger.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
205205
this.autocomplete._keyManager.onKeydown(event);
206206
if (event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW) {
207207
this.openPanel();
208-
this._scrollToOption();
208+
Promise.resolve().then(() => this._scrollToOption());
209209
}
210210
}
211211
}

src/lib/autocomplete/autocomplete.spec.ts

Lines changed: 156 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {TestBed, async, ComponentFixture} from '@angular/core/testing';
1+
import {TestBed, async, fakeAsync, tick, ComponentFixture} from '@angular/core/testing';
22
import {Component, OnDestroy, QueryList, ViewChild, ViewChildren} from '@angular/core';
33
import {By} from '@angular/platform-browser';
44
import {MdAutocompleteModule, MdAutocompleteTrigger} from './index';
@@ -523,97 +523,88 @@ describe('MdAutocomplete', () => {
523523
});
524524
}));
525525

526-
it('should set the active item to the first option when DOWN key is pressed', async(() => {
527-
fixture.whenStable().then(() => {
528-
const optionEls =
529-
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
526+
it('should set the active item to the first option when DOWN key is pressed', fakeAsync(() => {
527+
tick();
528+
const optionEls =
529+
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
530530

531-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
531+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
532+
tick();
533+
fixture.detectChanges();
532534

533-
fixture.whenStable().then(() => {
534-
fixture.detectChanges();
535-
expect(fixture.componentInstance.trigger.activeOption)
536-
.toBe(fixture.componentInstance.options.first, 'Expected first option to be active.');
537-
expect(optionEls[0].classList).toContain('mat-active');
538-
expect(optionEls[1].classList).not.toContain('mat-active');
535+
expect(fixture.componentInstance.trigger.activeOption)
536+
.toBe(fixture.componentInstance.options.first, 'Expected first option to be active.');
537+
expect(optionEls[0].classList).toContain('mat-active');
538+
expect(optionEls[1].classList).not.toContain('mat-active');
539539

540-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
540+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
541+
tick();
542+
fixture.detectChanges();
541543

542-
fixture.whenStable().then(() => {
543-
fixture.detectChanges();
544-
expect(fixture.componentInstance.trigger.activeOption)
545-
.toBe(fixture.componentInstance.options.toArray()[1],
546-
'Expected second option to be active.');
547-
expect(optionEls[0].classList).not.toContain('mat-active');
548-
expect(optionEls[1].classList).toContain('mat-active');
549-
});
550-
});
551-
});
544+
expect(fixture.componentInstance.trigger.activeOption)
545+
.toBe(fixture.componentInstance.options.toArray()[1],
546+
'Expected second option to be active.');
547+
expect(optionEls[0].classList).not.toContain('mat-active');
548+
expect(optionEls[1].classList).toContain('mat-active');
552549
}));
553550

554-
it('should set the active item to the last option when UP key is pressed', async(() => {
555-
fixture.whenStable().then(() => {
556-
const optionEls =
557-
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
551+
it('should set the active item to the last option when UP key is pressed', fakeAsync(() => {
552+
tick();
553+
const optionEls =
554+
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
558555

559-
const UP_ARROW_EVENT = new MockKeyboardEvent(UP_ARROW) as KeyboardEvent;
560-
fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
556+
const UP_ARROW_EVENT = new MockKeyboardEvent(UP_ARROW) as KeyboardEvent;
557+
fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
558+
tick();
559+
fixture.detectChanges();
561560

562-
fixture.whenStable().then(() => {
563-
fixture.detectChanges();
564-
expect(fixture.componentInstance.trigger.activeOption)
565-
.toBe(fixture.componentInstance.options.last, 'Expected last option to be active.');
566-
expect(optionEls[10].classList).toContain('mat-active');
567-
expect(optionEls[0].classList).not.toContain('mat-active');
561+
expect(fixture.componentInstance.trigger.activeOption)
562+
.toBe(fixture.componentInstance.options.last, 'Expected last option to be active.');
563+
expect(optionEls[10].classList).toContain('mat-active');
564+
expect(optionEls[0].classList).not.toContain('mat-active');
568565

569-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
566+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
567+
tick();
568+
fixture.detectChanges();
570569

571-
fixture.whenStable().then(() => {
572-
fixture.detectChanges();
573-
expect(fixture.componentInstance.trigger.activeOption)
574-
.toBe(fixture.componentInstance.options.first,
575-
'Expected first option to be active.');
576-
expect(optionEls[0].classList).toContain('mat-active');
577-
expect(optionEls[10].classList).not.toContain('mat-active');
578-
});
579-
});
580-
});
570+
expect(fixture.componentInstance.trigger.activeOption)
571+
.toBe(fixture.componentInstance.options.first,
572+
'Expected first option to be active.');
573+
expect(optionEls[0].classList).toContain('mat-active');
581574
}));
582575

583-
it('should set the active item properly after filtering', async(() => {
584-
fixture.whenStable().then(() => {
585-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
586-
fixture.detectChanges();
576+
it('should set the active item properly after filtering', fakeAsync(() => {
577+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
578+
tick();
579+
fixture.detectChanges();
587580

588-
fixture.whenStable().then(() => {
589-
typeInElement('o', input);
590-
fixture.detectChanges();
581+
typeInElement('o', input);
582+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
583+
tick();
584+
fixture.detectChanges();
591585

592-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
586+
const optionEls =
587+
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
593588

594-
fixture.whenStable().then(() => {
595-
fixture.detectChanges();
596-
const optionEls =
597-
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
598-
599-
expect(fixture.componentInstance.trigger.activeOption)
600-
.toBe(fixture.componentInstance.options.first,
601-
'Expected first option to be active.');
602-
expect(optionEls[0].classList).toContain('mat-active');
603-
expect(optionEls[1].classList).not.toContain('mat-active');
604-
});
605-
});
606-
});
589+
expect(fixture.componentInstance.trigger.activeOption)
590+
.toBe(fixture.componentInstance.options.first,
591+
'Expected first option to be active.');
592+
expect(optionEls[0].classList).toContain('mat-active');
593+
expect(optionEls[1].classList).not.toContain('mat-active');
607594
}));
608595

609596
it('should fill the text field when an option is selected with ENTER', async(() => {
610597
fixture.whenStable().then(() => {
611598
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
612-
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
613599

614-
fixture.detectChanges();
615-
expect(input.value)
616-
.toContain('Alabama', `Expected text field to fill with selected value on ENTER.`);
600+
fixture.whenStable().then(() => {
601+
fixture.detectChanges();
602+
603+
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
604+
fixture.detectChanges();
605+
expect(input.value)
606+
.toContain('Alabama', `Expected text field to fill with selected value on ENTER.`);
607+
});
617608
});
618609
}));
619610

@@ -624,11 +615,16 @@ describe('MdAutocomplete', () => {
624615

625616
const SPACE_EVENT = new MockKeyboardEvent(SPACE) as KeyboardEvent;
626617
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
627-
fixture.componentInstance.trigger._handleKeydown(SPACE_EVENT);
628-
fixture.detectChanges();
629618

630-
expect(input.value)
631-
.not.toContain('New York', `Expected option not to be selected on SPACE.`);
619+
fixture.whenStable().then(() => {
620+
fixture.detectChanges();
621+
622+
fixture.componentInstance.trigger._handleKeydown(SPACE_EVENT);
623+
fixture.detectChanges();
624+
625+
expect(input.value)
626+
.not.toContain('New York', `Expected option not to be selected on SPACE.`);
627+
});
632628
});
633629
}));
634630

@@ -638,54 +634,74 @@ describe('MdAutocomplete', () => {
638634
.toBe(false, `Expected control to start out pristine.`);
639635

640636
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
641-
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
642-
fixture.detectChanges();
637+
fixture.whenStable().then(() => {
638+
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
639+
fixture.detectChanges();
643640

644-
expect(fixture.componentInstance.stateCtrl.dirty)
645-
.toBe(true, `Expected control to become dirty when option was selected by ENTER.`);
641+
expect(fixture.componentInstance.stateCtrl.dirty)
642+
.toBe(true, `Expected control to become dirty when option was selected by ENTER.`);
643+
});
646644
});
647645
}));
648646

649647
it('should open the panel again when typing after making a selection', async(() => {
650648
fixture.whenStable().then(() => {
651649
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
652-
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
653-
fixture.detectChanges();
650+
fixture.whenStable().then(() => {
651+
fixture.componentInstance.trigger._handleKeydown(ENTER_EVENT);
652+
fixture.detectChanges();
654653

655-
expect(fixture.componentInstance.trigger.panelOpen)
656-
.toBe(false, `Expected panel state to read closed after ENTER key.`);
657-
expect(overlayContainerElement.textContent)
658-
.toEqual('', `Expected panel to close after ENTER key.`);
654+
expect(fixture.componentInstance.trigger.panelOpen)
655+
.toBe(false, `Expected panel state to read closed after ENTER key.`);
656+
expect(overlayContainerElement.textContent)
657+
.toEqual('', `Expected panel to close after ENTER key.`);
659658

660-
typeInElement('Alabama', input);
661-
fixture.detectChanges();
659+
typeInElement('Alabama', input);
660+
fixture.detectChanges();
662661

663-
expect(fixture.componentInstance.trigger.panelOpen)
664-
.toBe(true, `Expected panel state to read open when typing in input.`);
665-
expect(overlayContainerElement.textContent)
666-
.toContain('Alabama', `Expected panel to display when typing in input.`);
662+
expect(fixture.componentInstance.trigger.panelOpen)
663+
.toBe(true, `Expected panel state to read open when typing in input.`);
664+
expect(overlayContainerElement.textContent)
665+
.toContain('Alabama', `Expected panel to display when typing in input.`);
666+
});
667667
});
668668
}));
669669

670-
it('should scroll to active options below the fold', async(() => {
671-
fixture.whenStable().then(() => {
672-
const scrollContainer = document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel');
670+
it('should scroll to active options below the fold', fakeAsync(() => {
671+
tick();
672+
const scrollContainer =
673+
document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel');
673674

675+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
676+
tick();
677+
fixture.detectChanges();
678+
expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to scroll.`);
679+
680+
// These down arrows will set the 6th option active, below the fold.
681+
[1, 2, 3, 4, 5].forEach(() => {
674682
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
675-
fixture.detectChanges();
676-
expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to scroll.`);
683+
tick();
684+
});
677685

678-
// These down arrows will set the 6th option active, below the fold.
679-
[1, 2, 3, 4, 5].forEach(() => {
680-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
681-
});
682-
fixture.detectChanges();
686+
// Expect option bottom minus the panel height (288 - 256 = 32)
687+
expect(scrollContainer.scrollTop)
688+
.toEqual(32, `Expected panel to reveal the sixth option.`);
689+
}));
683690

684-
// Expect option bottom minus the panel height (288 - 256 = 32)
685-
expect(scrollContainer.scrollTop).toEqual(32, `Expected panel to reveal the sixth option.`);
686-
});
691+
it('should scroll to active options on UP arrow', fakeAsync(() => {
692+
tick();
693+
const scrollContainer =
694+
document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel');
695+
696+
const UP_ARROW_EVENT = new MockKeyboardEvent(UP_ARROW) as KeyboardEvent;
697+
fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
698+
tick();
699+
fixture.detectChanges();
687700

701+
// Expect option bottom minus the panel height (528 - 256 = 272)
702+
expect(scrollContainer.scrollTop).toEqual(272, `Expected panel to reveal last option.`);
688703
}));
704+
689705
});
690706

691707
describe('aria', () => {
@@ -733,18 +749,23 @@ describe('MdAutocomplete', () => {
733749

734750
const DOWN_ARROW_EVENT = new MockKeyboardEvent(DOWN_ARROW) as KeyboardEvent;
735751
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
736-
fixture.detectChanges();
737752

738-
expect(input.getAttribute('aria-activedescendant'))
739-
.toEqual(fixture.componentInstance.options.first.id,
740-
'Expected aria-activedescendant to match the active item after 1 down arrow.');
753+
fixture.whenStable().then(() => {
754+
fixture.detectChanges();
755+
expect(input.getAttribute('aria-activedescendant'))
756+
.toEqual(fixture.componentInstance.options.first.id,
757+
'Expected aria-activedescendant to match the active item after 1 down arrow.');
741758

742-
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
743-
fixture.detectChanges();
759+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
760+
fixture.whenStable().then(() => {
761+
fixture.detectChanges();
762+
763+
expect(input.getAttribute('aria-activedescendant'))
764+
.toEqual(fixture.componentInstance.options.toArray()[1].id,
765+
'Expected aria-activedescendant to match the active item after 2 down arrows.');
766+
});
767+
});
744768

745-
expect(input.getAttribute('aria-activedescendant'))
746-
.toEqual(fixture.componentInstance.options.toArray()[1].id,
747-
'Expected aria-activedescendant to match the active item after 2 down arrows.');
748769
});
749770
}));
750771

@@ -896,6 +917,26 @@ describe('MdAutocomplete', () => {
896917
.toContain('Two', `Expected panel to display when input is focused.`);
897918
});
898919

920+
it('should filter properly with ngIf after setting the active item', fakeAsync(() => {
921+
const fixture = TestBed.createComponent(NgIfAutocomplete);
922+
fixture.detectChanges();
923+
924+
fixture.componentInstance.trigger.openPanel();
925+
tick();
926+
fixture.detectChanges();
927+
928+
const DOWN_ARROW_EVENT = new MockKeyboardEvent(DOWN_ARROW) as KeyboardEvent;
929+
fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
930+
tick();
931+
fixture.detectChanges();
932+
933+
const input = fixture.debugElement.query(By.css('input')).nativeElement;
934+
typeInElement('o', input);
935+
fixture.detectChanges();
936+
937+
expect(fixture.componentInstance.mdOptions.length).toBe(2);
938+
}));
939+
899940
});
900941
});
901942

@@ -973,9 +1014,10 @@ class NgIfAutocomplete {
9731014
optionCtrl = new FormControl();
9741015
filteredOptions: Observable<any>;
9751016
isVisible = true;
1017+
options = ['One', 'Two', 'Three'];
9761018

9771019
@ViewChild(MdAutocompleteTrigger) trigger: MdAutocompleteTrigger;
978-
options = ['One', 'Two', 'Three'];
1020+
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;
9791021

9801022
constructor() {
9811023
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {

src/lib/core/a11y/activedescendant-key-manager.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@ export class ActiveDescendantKeyManager extends ListKeyManager<Highlightable> {
2323
* styles from the previously active item.
2424
*/
2525
setActiveItem(index: number): void {
26-
if (this.activeItem) {
27-
this.activeItem.setInactiveStyles();
28-
}
29-
super.setActiveItem(index);
30-
if (this.activeItem) {
31-
this.activeItem.setActiveStyles();
32-
}
26+
Promise.resolve().then(() => {
27+
if (this.activeItem) {
28+
this.activeItem.setInactiveStyles();
29+
}
30+
super.setActiveItem(index);
31+
if (this.activeItem) {
32+
this.activeItem.setActiveStyles();
33+
}
34+
});
3335
}
3436

3537
}

0 commit comments

Comments
 (0)