Skip to content

Commit 36da4c3

Browse files
crisbetokara
authored andcommitted
refactor(autocomplete): make keyboard and visibility calls synchronous (#6441)
1 parent 6e3bbcb commit 36da4c3

File tree

7 files changed

+227
-313
lines changed

7 files changed

+227
-313
lines changed

src/cdk/a11y/activedescendant-key-manager.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ export class ActiveDescendantKeyManager<T> extends ListKeyManager<Highlightable
2626
* styles from the previously active item.
2727
*/
2828
setActiveItem(index: number): void {
29-
Promise.resolve().then(() => {
30-
if (this.activeItem) {
31-
this.activeItem.setInactiveStyles();
32-
}
33-
super.setActiveItem(index);
34-
if (this.activeItem) {
35-
this.activeItem.setActiveStyles();
36-
}
37-
});
29+
if (this.activeItem) {
30+
this.activeItem.setInactiveStyles();
31+
}
32+
super.setActiveItem(index);
33+
if (this.activeItem) {
34+
this.activeItem.setActiveStyles();
35+
}
3836
}
3937

4038
}

src/cdk/a11y/list-key-manager.spec.ts

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -550,13 +550,12 @@ describe('Key managers', () => {
550550
describe('ActiveDescendantKeyManager', () => {
551551
let keyManager: ActiveDescendantKeyManager<FakeHighlightable>;
552552

553-
beforeEach(fakeAsync(() => {
553+
beforeEach(() => {
554554
itemList.items = [new FakeHighlightable(), new FakeHighlightable(), new FakeHighlightable()];
555555
keyManager = new ActiveDescendantKeyManager<FakeHighlightable>(itemList);
556556

557557
// first item is already focused
558558
keyManager.setFirstItemActive();
559-
tick();
560559

561560
spyOn(itemList.items[0], 'setActiveStyles');
562561
spyOn(itemList.items[1], 'setActiveStyles');
@@ -565,44 +564,38 @@ describe('Key managers', () => {
565564
spyOn(itemList.items[0], 'setInactiveStyles');
566565
spyOn(itemList.items[1], 'setInactiveStyles');
567566
spyOn(itemList.items[2], 'setInactiveStyles');
568-
}));
567+
});
569568

570-
it('should set subsequent items as active with the DOWN arrow', fakeAsync(() => {
569+
it('should set subsequent items as active with the DOWN arrow', () => {
571570
keyManager.onKeydown(fakeKeyEvents.downArrow);
572-
tick();
573571

574572
expect(itemList.items[1].setActiveStyles).toHaveBeenCalled();
575573
expect(itemList.items[2].setActiveStyles).not.toHaveBeenCalled();
576574

577575
keyManager.onKeydown(fakeKeyEvents.downArrow);
578-
tick();
579576

580577
expect(itemList.items[2].setActiveStyles).toHaveBeenCalled();
581-
}));
578+
});
582579

583-
it('should set previous items as active with the UP arrow', fakeAsync(() => {
580+
it('should set previous items as active with the UP arrow', () => {
584581
keyManager.setLastItemActive();
585-
tick();
586-
587582
keyManager.onKeydown(fakeKeyEvents.upArrow);
588-
tick();
583+
589584
expect(itemList.items[1].setActiveStyles).toHaveBeenCalled();
590585
expect(itemList.items[0].setActiveStyles).not.toHaveBeenCalled();
591586

592587
keyManager.onKeydown(fakeKeyEvents.upArrow);
593-
tick();
588+
594589
expect(itemList.items[0].setActiveStyles).toHaveBeenCalled();
595-
}));
590+
});
596591

597-
it('should set inactive styles on previously active items', fakeAsync(() => {
592+
it('should set inactive styles on previously active items', () => {
598593
keyManager.onKeydown(fakeKeyEvents.downArrow);
599-
tick();
600594
expect(itemList.items[0].setInactiveStyles).toHaveBeenCalled();
601595

602596
keyManager.onKeydown(fakeKeyEvents.upArrow);
603-
tick();
604597
expect(itemList.items[1].setInactiveStyles).toHaveBeenCalled();
605-
}));
598+
});
606599

607600
});
608601

src/cdk/rxjs/rx-operators.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {startWith as startWithOperator} from 'rxjs/operator/startWith';
2222
import {debounceTime as debounceTimeOperator} from 'rxjs/operator/debounceTime';
2323
import {auditTime as auditTimeOperator} from 'rxjs/operator/auditTime';
2424
import {takeUntil as takeUntilOperator} from 'rxjs/operator/takeUntil';
25+
import {delay as delayOperator} from 'rxjs/operator/delay';
2526

2627
/**
2728
* Represents a strongly-typed chain of RxJS operators.
@@ -71,6 +72,9 @@ export interface StrictRxChain<T> {
7172

7273
call(operator: takeUntilOperatorType<T>, notifier: Observable<any>): StrictRxChain<T>;
7374

75+
call(operator: delayOperatorType<T>, delay: number | Date, scheduler?: IScheduler):
76+
StrictRxChain<T>;
77+
7478
subscribe(fn: (t: T) => void): Subscription;
7579

7680
result(): Observable<T>;
@@ -89,6 +93,7 @@ export class StartWithBrand { private _; }
8993
export class DebounceTimeBrand { private _; }
9094
export class AuditTimeBrand { private _; }
9195
export class TakeUntilBrand { private _; }
96+
export class DelayBrand { private _; }
9297
/* tslint:enable:no-unused-variable */
9398

9499

@@ -104,6 +109,7 @@ export type startWithOperatorType<T> = typeof startWithOperator & StartWithBrand
104109
export type debounceTimeOperatorType<T> = typeof debounceTimeOperator & DebounceTimeBrand;
105110
export type auditTimeOperatorType<T> = typeof auditTimeOperator & AuditTimeBrand;
106111
export type takeUntilOperatorType<T> = typeof takeUntilOperator & TakeUntilBrand;
112+
export type delayOperatorType<T> = typeof delayOperator & DelayBrand;
107113

108114
// We add `Function` to the type intersection to make this nomically different from
109115
// `finallyOperatorType` while still being structurally the same. Without this, TypeScript tries to
@@ -123,3 +129,4 @@ export const debounceTime =
123129
debounceTimeOperator as typeof debounceTimeOperator & DebounceTimeBrand & Function;
124130
export const auditTime = auditTimeOperator as typeof auditTimeOperator & AuditTimeBrand & Function;
125131
export const takeUntil = takeUntilOperator as typeof takeUntilOperator & TakeUntilBrand & Function;
132+
export const delay = delayOperator as typeof delayOperator & DelayBrand & Function;

src/lib/autocomplete/autocomplete-trigger.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
ScrollStrategy,
1919
} from '@angular/cdk/overlay';
2020
import {TemplatePortal} from '@angular/cdk/portal';
21-
import {filter, first, map, RxChain, switchMap} from '@angular/cdk/rxjs';
21+
import {filter, first, RxChain, switchMap, doOperator, delay} from '@angular/cdk/rxjs';
2222
import {
2323
ChangeDetectorRef,
2424
Directive,
@@ -284,11 +284,9 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
284284
this.openPanel();
285285
}
286286

287-
Promise.resolve().then(() => {
288-
if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
289-
this._scrollToOption();
290-
}
291-
});
287+
if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
288+
this._scrollToOption();
289+
}
292290
}
293291
}
294292

@@ -356,9 +354,8 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
356354
this.autocomplete._setScrollTop(optionOffset);
357355
} else if (optionOffset + AUTOCOMPLETE_OPTION_HEIGHT > panelTop + AUTOCOMPLETE_PANEL_HEIGHT) {
358356
// Scroll down to reveal selected option scrolled below the panel bottom
359-
const newScrollTop =
360-
Math.max(0, optionOffset - AUTOCOMPLETE_PANEL_HEIGHT + AUTOCOMPLETE_OPTION_HEIGHT);
361-
this.autocomplete._setScrollTop(newScrollTop);
357+
const newScrollTop = optionOffset - AUTOCOMPLETE_PANEL_HEIGHT + AUTOCOMPLETE_OPTION_HEIGHT;
358+
this.autocomplete._setScrollTop(Math.max(0, newScrollTop));
362359
}
363360
}
364361

@@ -368,8 +365,12 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
368365
*/
369366
private _subscribeToClosingActions(): Subscription {
370367
const firstStable = first.call(this._zone.onStable.asObservable());
371-
const optionChanges = map.call(this.autocomplete.options.changes, () =>
372-
this._positionStrategy.recalculateLastPosition());
368+
const optionChanges = RxChain.from(this.autocomplete.options.changes)
369+
.call(doOperator, () => this._positionStrategy.recalculateLastPosition())
370+
// Defer emitting to the stream until the next tick, because changing
371+
// bindings in here will cause "changed after checked" errors.
372+
.call(delay, 0)
373+
.result();
373374

374375
// When the zone is stable initially, and when the option list changes...
375376
return RxChain.from(merge(firstStable, optionChanges))

0 commit comments

Comments
 (0)