From 6bd9f4a9827e4641ef88e8a54a5a975c291f1c9d Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 19 Apr 2017 20:16:49 +0200 Subject: [PATCH] fix(select): prevent the panel from going outside the viewport horizontally Prevents the select panel from going outside the viewport along the x axis. Fixes #3504. Fixes #3831. --- src/lib/select/select.html | 2 +- src/lib/select/select.spec.ts | 185 ++++++++++++++++++++++++---------- src/lib/select/select.ts | 62 +++++++++--- 3 files changed, 180 insertions(+), 69 deletions(-) diff --git a/src/lib/select/select.html b/src/lib/select/select.html index 9c9f0eefac97..0d68551b00af 100644 --- a/src/lib/select/select.html +++ b/src/lib/select/select.html @@ -15,7 +15,7 @@ + [offsetY]="_offsetY" (attach)="_onAttached()">
diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index b53781ede656..5996c787648d 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -24,9 +24,22 @@ import {dispatchFakeEvent} from '../core/testing/dispatch-events'; import {wrappedErrorMessage} from '../core/testing/wrapped-error-message'; +class FakeViewportRuler { + getViewportRect() { + return { + left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014 + }; + } + + getViewportScrollPosition() { + return {top: 0, left: 0}; + } +} + describe('MdSelect', () => { let overlayContainerElement: HTMLElement; let dir: {value: 'ltr'|'rtl'}; + let fakeViewportRuler = new FakeViewportRuler(); beforeEach(async(() => { TestBed.configureTestingModule({ @@ -67,7 +80,7 @@ describe('MdSelect', () => { {provide: Dir, useFactory: () => { return dir = { value: 'ltr' }; }}, - {provide: ViewportRuler, useClass: FakeViewportRuler} + {provide: ViewportRuler, useValue: fakeViewportRuler} ] }); @@ -918,6 +931,91 @@ describe('MdSelect', () => { }); + describe('limited space to open horizontally', () => { + beforeEach(() => { + select.style.position = 'absolute'; + select.style.top = '200px'; + }); + + it('should stay within the viewport when overflowing on the left in ltr', fakeAsync(() => { + select.style.left = '-100px'; + trigger.click(); + tick(400); + fixture.detectChanges(); + + const panelLeft = document.querySelector('.mat-select-panel') + .getBoundingClientRect().left; + expect(panelLeft).toBeGreaterThan(0, + `Expected select panel to be inside the viewport in ltr.`); + })); + + it('should stay within the viewport when overflowing on the left in rtl', fakeAsync(() => { + dir.value = 'rtl'; + select.style.left = '-100px'; + trigger.click(); + tick(400); + fixture.detectChanges(); + + const panelLeft = document.querySelector('.mat-select-panel') + .getBoundingClientRect().left; + + expect(panelLeft).toBeGreaterThan(0, + `Expected select panel to be inside the viewport in rtl.`); + })); + + it('should stay within the viewport when overflowing on the right in ltr', fakeAsync(() => { + select.style.right = '-100px'; + trigger.click(); + tick(400); + fixture.detectChanges(); + + const viewportRect = fakeViewportRuler.getViewportRect().right; + const panelRight = document.querySelector('.mat-select-panel') + .getBoundingClientRect().right; + + expect(viewportRect - panelRight).toBeGreaterThan(0, + `Expected select panel to be inside the viewport in ltr.`); + })); + + it('should stay within the viewport when overflowing on the right in rtl', fakeAsync(() => { + dir.value = 'rtl'; + select.style.right = '-100px'; + trigger.click(); + tick(400); + fixture.detectChanges(); + + const viewportRect = fakeViewportRuler.getViewportRect().right; + const panelRight = document.querySelector('.mat-select-panel') + .getBoundingClientRect().right; + + expect(viewportRect - panelRight).toBeGreaterThan(0, + `Expected select panel to be inside the viewport in rtl.`); + })); + + it('should keep the position within the viewport on repeat openings', async(() => { + select.style.left = '-100px'; + trigger.click(); + fixture.detectChanges(); + + let panelLeft = document.querySelector('.mat-select-panel').getBoundingClientRect().left; + + expect(panelLeft).toBeGreaterThan(0, `Expected select panel to be inside the viewport.`); + + fixture.componentInstance.select.close(); + fixture.detectChanges(); + + fixture.whenStable().then(() => { + trigger.click(); + fixture.detectChanges(); + panelLeft = document.querySelector('.mat-select-panel').getBoundingClientRect().left; + + expect(panelLeft).toBeGreaterThan(0, + `Expected select panel continue being inside the viewport.`); + }); + })); + + }); + describe('when scrolled', () => { // Need to set the scrollTop two different ways to support @@ -1024,42 +1122,38 @@ describe('MdSelect', () => { select.style.marginRight = '30px'; }); - it('should align the trigger and the selected option on the x-axis in ltr', async(() => { + it('should align the trigger and the selected option on the x-axis in ltr', fakeAsync(() => { trigger.click(); + tick(400); fixture.detectChanges(); - fixture.whenStable().then(() => { - const triggerLeft = trigger.getBoundingClientRect().left; - const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option') - .getBoundingClientRect().left; + const triggerLeft = trigger.getBoundingClientRect().left; + const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option') + .getBoundingClientRect().left; - // Each option is 32px wider than the trigger, so it must be adjusted 16px - // to ensure the text overlaps correctly. - expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2), - `Expected trigger to align with the selected option on the x-axis in LTR.`); - }); + // Each option is 32px wider than the trigger, so it must be adjusted 16px + // to ensure the text overlaps correctly. + expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2), + `Expected trigger to align with the selected option on the x-axis in LTR.`); })); - it('should align the trigger and the selected option on the x-axis in rtl', async(() => { + it('should align the trigger and the selected option on the x-axis in rtl', fakeAsync(() => { dir.value = 'rtl'; - fixture.whenStable().then(() => { - fixture.detectChanges(); + fixture.detectChanges(); - trigger.click(); - fixture.detectChanges(); + trigger.click(); + tick(400); + fixture.detectChanges(); - fixture.whenStable().then(() => { - const triggerRight = trigger.getBoundingClientRect().right; - const firstOptionRight = - document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; - - // Each option is 32px wider than the trigger, so it must be adjusted 16px - // to ensure the text overlaps correctly. - expect(firstOptionRight.toFixed(2)) - .toEqual((triggerRight + 16).toFixed(2), - `Expected trigger to align with the selected option on the x-axis in RTL.`); - }); - }); + const triggerRight = trigger.getBoundingClientRect().right; + const firstOptionRight = + document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; + + // Each option is 32px wider than the trigger, so it must be adjusted 16px + // to ensure the text overlaps correctly. + expect(firstOptionRight.toFixed(2)) + .toEqual((triggerRight + 16).toFixed(2), + `Expected trigger to align with the selected option on the x-axis in RTL.`); })); }); @@ -1072,8 +1166,8 @@ describe('MdSelect', () => { trigger = multiFixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; select = multiFixture.debugElement.query(By.css('md-select')).nativeElement; - select.style.marginLeft = '20px'; - select.style.marginRight = '20px'; + select.style.marginLeft = '60px'; + select.style.marginRight = '60px'; }); it('should adjust for the checkbox in ltr', async(() => { @@ -1092,21 +1186,20 @@ describe('MdSelect', () => { }); })); - it('should adjust for the checkbox in rtl', async(() => { + it('should adjust for the checkbox in rtl', fakeAsync(() => { dir.value = 'rtl'; trigger.click(); + tick(400); multiFixture.detectChanges(); - multiFixture.whenStable().then(() => { - const triggerRight = trigger.getBoundingClientRect().right; - const firstOptionRight = - document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; + const triggerRight = trigger.getBoundingClientRect().right; + const firstOptionRight = + document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; - // 48px accounts for the checkbox size, margin and the panel's padding. - expect(firstOptionRight.toFixed(2)) - .toEqual((triggerRight + 48).toFixed(2), - `Expected trigger label to align along x-axis, accounting for the checkbox.`); - }); + // 48px accounts for the checkbox size, margin and the panel's padding. + expect(firstOptionRight.toFixed(2)) + .toEqual((triggerRight + 48).toFixed(2), + `Expected trigger label to align along x-axis, accounting for the checkbox.`); })); }); @@ -2024,15 +2117,3 @@ class BasicSelectInitiallyHidden { ` }) class BasicSelectNoPlaceholder { } - -class FakeViewportRuler { - getViewportRect() { - return { - left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014 - }; - } - - getViewportScrollPosition() { - return {top: 0, left: 0}; - } -} diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 55dd43557a05..8146fc636149 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -187,13 +187,6 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal /** Whether the panel's animation is done. */ _panelDoneAnimating: boolean = false; - /** - * The x-offset of the overlay panel in relation to the trigger's top start corner. - * This must be adjusted to align the selected option text over the trigger text when - * the panel opens. Will change based on LTR or RTL text direction. - */ - _offsetX = 0; - /** * The y-offset of the overlay panel in relation to the trigger's top start corner. * This must be adjusted to align the selected option text over the trigger text. @@ -474,6 +467,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal } else { this.onClose.emit(); this._panelDoneAnimating = false; + this.overlayDir.offsetX = 0; } } @@ -495,12 +489,20 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal } } + /** + * Callback that is invoked when the overlay panel has been attached. + */ + _onAttached(): void { + this._calculateOverlayOffsetX(); + this._setScrollTop(); + } + /** * Sets the scroll position of the scroll container. This must be called after * the overlay pane is attached or the scroll container element will not yet be * present in the DOM. */ - _setScrollTop(): void { + private _setScrollTop(): void { const scrollContainer = this.overlayDir.overlayRef.overlayElement.querySelector('.mat-select-panel'); scrollContainer.scrollTop = this._scrollTop; @@ -698,12 +700,6 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal /** Calculates the scroll position and x- and y-offsets of the overlay panel. */ private _calculateOverlayPosition(): void { - this._offsetX = this.multiple ? SELECT_MULTIPLE_PANEL_PADDING_X : SELECT_PANEL_PADDING_X; - - if (!this._isRtl()) { - this._offsetX *= -1; - } - const panelHeight = Math.min(this.options.length * SELECT_OPTION_HEIGHT, SELECT_PANEL_MAX_HEIGHT); const scrollContainerHeight = this.options.length * SELECT_OPTION_HEIGHT; @@ -717,7 +713,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal // center of the overlay panel rather than the top. const scrollBuffer = panelHeight / 2; this._scrollTop = this._calculateOverlayScroll(selectedIndex, scrollBuffer, maxScroll); - this._offsetY = this._calculateOverlayOffset(selectedIndex, scrollBuffer, maxScroll); + this._offsetY = this._calculateOverlayOffsetY(selectedIndex, scrollBuffer, maxScroll); } else { // If no option is selected, the panel centers on the first option. In this case, // we must only adjust for the height difference between the option element @@ -779,12 +775,46 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal return this.ariaLabelledby ? null : this.ariaLabel || this.placeholder; } + /** + * Sets the x-offset of the overlay panel in relation to the trigger's top start corner. + * This must be adjusted to align the selected option text over the trigger text when + * the panel opens. Will change based on LTR or RTL text direction. Note that the offset + * can't be calculated until the panel has been attached, because we need to know the + * content width in order to constrain the panel within the viewport. + */ + private _calculateOverlayOffsetX(): void { + const overlayRect = this.overlayDir.overlayRef.overlayElement.getBoundingClientRect(); + const viewportRect = this._viewportRuler.getViewportRect(); + const isRtl = this._isRtl(); + let offsetX = this.multiple ? SELECT_MULTIPLE_PANEL_PADDING_X : SELECT_PANEL_PADDING_X; + + if (!isRtl) { + offsetX *= -1; + } + + const leftOverflow = 0 - (overlayRect.left + offsetX + - (isRtl ? SELECT_PANEL_PADDING_X * 2 : 0)); + const rightOverflow = overlayRect.right + offsetX - viewportRect.width + + (isRtl ? 0 : SELECT_PANEL_PADDING_X * 2); + + if (leftOverflow > 0) { + offsetX += leftOverflow + SELECT_PANEL_VIEWPORT_PADDING; + } else if (rightOverflow > 0) { + offsetX -= rightOverflow + SELECT_PANEL_VIEWPORT_PADDING; + } + + // Set the offset directly in order to avoid having to go through change detection and + // potentially triggering "changed after it was checked" errors. + this.overlayDir.offsetX = offsetX; + this.overlayDir.overlayRef.updatePosition(); + } + /** * Calculates the y-offset of the select's overlay panel in relation to the * top start corner of the trigger. It has to be adjusted in order for the * selected option to be aligned over the trigger when the panel opens. */ - private _calculateOverlayOffset(selectedIndex: number, scrollBuffer: number, + private _calculateOverlayOffsetY(selectedIndex: number, scrollBuffer: number, maxScroll: number): number { let optionOffsetFromPanelTop: number;