From 73a865592c9cdf34f74756ad332c59cbf6002f7b Mon Sep 17 00:00:00 2001 From: Will Howell Date: Tue, 27 Jun 2017 20:07:24 -0400 Subject: [PATCH] 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 --- src/lib/autocomplete/autocomplete-trigger.ts | 21 +++++-- src/lib/autocomplete/autocomplete.spec.ts | 62 +++++++++++++++++++- src/lib/autocomplete/autocomplete.ts | 9 ++- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index da2b43935c0e..1573b17a2bd7 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/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); + } } /** diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts index 699b1f6bd568..4f47b42fd7d1 100644 --- a/src/lib/autocomplete/autocomplete.spec.ts +++ b/src/lib/autocomplete/autocomplete.spec.ts @@ -548,6 +548,7 @@ describe('MdAutocomplete', () => { let fixture: ComponentFixture; 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; - 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); diff --git a/src/lib/autocomplete/autocomplete.ts b/src/lib/autocomplete/autocomplete.ts index 081682eba353..03939f6b7b53 100644 --- a/src/lib/autocomplete/autocomplete.ts +++ b/src/lib/autocomplete/autocomplete.ts @@ -73,8 +73,8 @@ 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) { @@ -82,6 +82,11 @@ export class MdAutocomplete implements AfterContentInit { } } + /** 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(() => {