fix(autocomplete): don't scroll panel when option is visible (#4905)

* fix(autocomplete): don't scroll panel if option is visible

* Clean up comments & indent by 4 on line continuations

Author: willshowell

src/lib/autocomplete/autocomplete-trigger.ts

@@ -321,15 +321,26 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
   /**
    * Given that we are not actually focusing active options, we must manually adjust scroll
    * to reveal options below the fold. First, we find the offset of the option from the top
-   * of the panel. The new scrollTop will be that offset - the panel height + the option
-   * height, so the active option will be just visible at the bottom of the panel.
+   * of the panel. If that offset is below the fold, the new scrollTop will be the offset -
+   * the panel height + the option height, so the active option will be just visible at the
+   * bottom of the panel. If that offset is above the top of the visible panel, the new scrollTop
+   * will become the offset. If that offset is visible within the panel already, the scrollTop is
+   * not adjusted.
    */
   private _scrollToOption(): void {
     const optionOffset = this.autocomplete._keyManager.activeItemIndex ?
         this.autocomplete._keyManager.activeItemIndex * AUTOCOMPLETE_OPTION_HEIGHT : 0;
-    const newScrollTop =
-        Math.max(0, optionOffset - AUTOCOMPLETE_PANEL_HEIGHT + AUTOCOMPLETE_OPTION_HEIGHT);
-    this.autocomplete._setScrollTop(newScrollTop);
+    const panelTop = this.autocomplete._getScrollTop();
+
+    if (optionOffset < panelTop) {
+      // Scroll up to reveal selected option scrolled above the panel top
+      this.autocomplete._setScrollTop(optionOffset);
+    } else if (optionOffset + AUTOCOMPLETE_OPTION_HEIGHT > panelTop + AUTOCOMPLETE_PANEL_HEIGHT) {
+      // Scroll down to reveal selected option scrolled below the panel bottom
+      const newScrollTop =
+          Math.max(0, optionOffset - AUTOCOMPLETE_PANEL_HEIGHT + AUTOCOMPLETE_OPTION_HEIGHT);
+      this.autocomplete._setScrollTop(newScrollTop);
+    }
   }
 
   /**

src/lib/autocomplete/autocomplete.spec.ts

@@ -548,6 +548,7 @@ describe('MdAutocomplete', () => {
     let fixture: ComponentFixture<SimpleAutocomplete>;
     let input: HTMLInputElement;
     let DOWN_ARROW_EVENT: KeyboardEvent;
+    let UP_ARROW_EVENT: KeyboardEvent;
     let ENTER_EVENT: KeyboardEvent;
 
     beforeEach(() => {
@@ -556,6 +557,7 @@ describe('MdAutocomplete', () => {
 
       input = fixture.debugElement.query(By.css('input')).nativeElement;
       DOWN_ARROW_EVENT = createKeyboardEvent('keydown', DOWN_ARROW);
+      UP_ARROW_EVENT = createKeyboardEvent('keydown', UP_ARROW);
       ENTER_EVENT = createKeyboardEvent('keydown', ENTER);
 
       fixture.componentInstance.trigger.openPanel();
@@ -614,7 +616,6 @@ describe('MdAutocomplete', () => {
       const optionEls =
           overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
 
-      const UP_ARROW_EVENT = createKeyboardEvent('keydown', UP_ARROW);
       fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
       tick();
       fixture.detectChanges();
@@ -768,7 +769,6 @@ describe('MdAutocomplete', () => {
       const scrollContainer =
           document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel')!;
 
-      const UP_ARROW_EVENT = createKeyboardEvent('keydown', UP_ARROW);
       fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
       tick();
       fixture.detectChanges();
@@ -777,6 +777,64 @@ describe('MdAutocomplete', () => {
       expect(scrollContainer.scrollTop).toEqual(272, `Expected panel to reveal last option.`);
     }));
 
+    it('should not scroll to active options that are fully in the panel', fakeAsync(() => {
+      tick();
+      const scrollContainer =
+          document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel')!;
+
+      fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
+      tick();
+      fixture.detectChanges();
+      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(() => {
+        fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
+        tick();
+      });
+
+      // Expect option bottom minus the panel height (288 - 256 = 32)
+      expect(scrollContainer.scrollTop)
+          .toEqual(32, `Expected panel to reveal the sixth option.`);
+
+      // These up arrows will set the 2nd option active
+      [4, 3, 2, 1].forEach(() => {
+        fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
+        tick();
+      });
+
+      // Expect no scrolling to have occurred. Still showing bottom of 6th option.
+      expect(scrollContainer.scrollTop)
+          .toEqual(32, `Expected panel not to scroll up since sixth option still fully visible.`);
+    }));
+
+    it('should scroll to active options that are above the panel', fakeAsync(() => {
+      tick();
+      const scrollContainer =
+          document.querySelector('.cdk-overlay-pane .mat-autocomplete-panel')!;
+
+      fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
+      tick();
+      fixture.detectChanges();
+      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(() => {
+        fixture.componentInstance.trigger._handleKeydown(DOWN_ARROW_EVENT);
+        tick();
+      });
+
+      // These up arrows will set the 2nd option active
+      [5, 4, 3, 2, 1].forEach(() => {
+        fixture.componentInstance.trigger._handleKeydown(UP_ARROW_EVENT);
+        tick();
+      });
+
+      // Expect to show the top of the 2nd option at the top of the panel
+      expect(scrollContainer.scrollTop)
+          .toEqual(48, `Expected panel to scroll up when option is above panel.`);
+    }));
+
     it('should close the panel when pressing escape', async(() => {
       const trigger = fixture.componentInstance.trigger;
       const escapeEvent = createKeyboardEvent('keydown', ESCAPE);

src/lib/autocomplete/autocomplete.ts

@@ -73,15 +73,20 @@ export class MdAutocomplete implements AfterContentInit {
   }
 
   /**
-   * Sets the panel scrollTop. This allows us to manually scroll to display
-   * options below the fold, as they are not actually being focused when active.
+   * Sets the panel scrollTop. This allows us to manually scroll to display options
+   * above or below the fold, as they are not actually being focused when active.
    */
   _setScrollTop(scrollTop: number): void {
     if (this.panel) {
       this.panel.nativeElement.scrollTop = scrollTop;
     }
   }
 
+  /** Returns the panel's scrollTop. */
+  _getScrollTop(): number {
+    return this.panel ? this.panel.nativeElement.scrollTop : 0;
+  }
+
   /** Panel should hide itself when the option list is empty. */
   _setVisibility() {
     Promise.resolve().then(() => {