refactor(autocomplete): make keyboard and visibility calls synchronous

Reworks a few methods, that were changing the view in a `Promise.resolve`, to be synchronous. The initial idea behind deferring those calls was to avoid "changed after checked" errors, however they also make things harder to test. It seems like the root of the issue for all of them was that we were trying to modify view properties after the `options.changed` callback had fired, which comes after change detection. These changes have the advantage of making things a lot easier and cleaner to test, as well as making the API easier to use.

Author: crisbeto

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

@@ -26,15 +26,13 @@ export class ActiveDescendantKeyManager<T> extends ListKeyManager<Highlightable
    * styles from the previously active item.
    */
   setActiveItem(index: number): void {
-    Promise.resolve().then(() => {
-      if (this.activeItem) {
-        this.activeItem.setInactiveStyles();
-      }
-      super.setActiveItem(index);
-      if (this.activeItem) {
-        this.activeItem.setActiveStyles();
-      }
-    });
+    if (this.activeItem) {
+      this.activeItem.setInactiveStyles();
+    }
+    super.setActiveItem(index);
+    if (this.activeItem) {
+      this.activeItem.setActiveStyles();
+    }
   }
 
 }

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

@@ -540,13 +540,12 @@ describe('Key managers', () => {
   describe('ActiveDescendantKeyManager', () => {
     let keyManager: ActiveDescendantKeyManager<FakeHighlightable>;
 
-    beforeEach(fakeAsync(() => {
+    beforeEach(() => {
       itemList.items = [new FakeHighlightable(), new FakeHighlightable(), new FakeHighlightable()];
       keyManager = new ActiveDescendantKeyManager<FakeHighlightable>(itemList);
 
       // first item is already focused
       keyManager.setFirstItemActive();
-      tick();
 
       spyOn(itemList.items[0], 'setActiveStyles');
       spyOn(itemList.items[1], 'setActiveStyles');
@@ -555,44 +554,38 @@ describe('Key managers', () => {
       spyOn(itemList.items[0], 'setInactiveStyles');
       spyOn(itemList.items[1], 'setInactiveStyles');
       spyOn(itemList.items[2], 'setInactiveStyles');
-    }));
+    });
 
-    it('should set subsequent items as active with the DOWN arrow', fakeAsync(() => {
+    it('should set subsequent items as active with the DOWN arrow', () => {
       keyManager.onKeydown(fakeKeyEvents.downArrow);
-      tick();
 
       expect(itemList.items[1].setActiveStyles).toHaveBeenCalled();
       expect(itemList.items[2].setActiveStyles).not.toHaveBeenCalled();
 
       keyManager.onKeydown(fakeKeyEvents.downArrow);
-      tick();
 
       expect(itemList.items[2].setActiveStyles).toHaveBeenCalled();
-    }));
+    });
 
-    it('should set previous items as active with the UP arrow', fakeAsync(() => {
+    it('should set previous items as active with the UP arrow', () => {
       keyManager.setLastItemActive();
-      tick();
-
       keyManager.onKeydown(fakeKeyEvents.upArrow);
-      tick();
+
       expect(itemList.items[1].setActiveStyles).toHaveBeenCalled();
       expect(itemList.items[0].setActiveStyles).not.toHaveBeenCalled();
 
       keyManager.onKeydown(fakeKeyEvents.upArrow);
-      tick();
+
       expect(itemList.items[0].setActiveStyles).toHaveBeenCalled();
-    }));
+    });
 
-    it('should set inactive styles on previously active items', fakeAsync(() => {
+    it('should set inactive styles on previously active items', () => {
       keyManager.onKeydown(fakeKeyEvents.downArrow);
-      tick();
       expect(itemList.items[0].setInactiveStyles).toHaveBeenCalled();
 
       keyManager.onKeydown(fakeKeyEvents.upArrow);
-      tick();
       expect(itemList.items[1].setInactiveStyles).toHaveBeenCalled();
-    }));
+    });
 
   });
 

src/lib/autocomplete/autocomplete-trigger.ts

@@ -23,7 +23,7 @@ import {
 import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
 import {DOCUMENT} from '@angular/platform-browser';
 import {Directionality} from '@angular/cdk/bidi';
-import {filter, first, map, RxChain, switchMap} from '@angular/cdk/rxjs';
+import {filter, first, RxChain, switchMap} from '@angular/cdk/rxjs';
 import {
   ConnectedPositionStrategy,
   Overlay,
@@ -291,11 +291,9 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
         this.openPanel();
       }
 
-      Promise.resolve().then(() => {
-        if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
-          this._scrollToOption();
-        }
-      });
+      if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
+        this._scrollToOption();
+      }
     }
   }
 
@@ -373,8 +371,13 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
    */
   private _subscribeToClosingActions(): Subscription {
     const firstStable = first.call(this._zone.onStable);
-    const optionChanges = map.call(this.autocomplete.options.changes, () =>
-      this._positionStrategy.recalculateLastPosition());
+    const optionChanges = switchMap.call(this.autocomplete.options.changes, () => {
+      this._positionStrategy.recalculateLastPosition();
+
+      // Defer emitting to the stream until the next tick, because changing
+      // bindings in here will cause "changed after checked" errors.
+      return Promise.resolve();
+    });
 
     // When the zone is stable initially, and when the option list changes...
     return RxChain.from(merge(firstStable, optionChanges))