From fa260cac41a3456e50c8bd859b30782a64f46a16 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sun, 23 Apr 2017 17:52:12 +0200 Subject: [PATCH] fix(connected-overlay): better handling of dynamic content * Refactors the `ConnectedPositionStrategy` to be able to use `right` and `bottom` to position the panel. This will allow the element to keep its position automatically, even if the element's size changes. * Removes the uses of the `FakeViewportRuler` in some of the unit tests since it doesn't work very well when the element is positioned relatively to the right edge of the screen. * Rounds down the values when testing positioning. This is necessary, because some browsers (particularly Chrome on Windows) can have some subpixel deviations. Fixes #4155. --- src/lib/autocomplete/autocomplete.spec.ts | 21 +-- src/lib/core/overlay/_overlay.scss | 6 +- .../connected-position-strategy.spec.ts | 146 ++++++------------ .../position/connected-position-strategy.ts | 69 +++++++-- src/lib/menu/menu.spec.ts | 78 ++++------ src/lib/select/select.spec.ts | 65 ++++---- 6 files changed, 178 insertions(+), 207 deletions(-) diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts index 78ccfe632da9..e1254f998370 100644 --- a/src/lib/autocomplete/autocomplete.spec.ts +++ b/src/lib/autocomplete/autocomplete.spec.ts @@ -18,8 +18,6 @@ import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; import {Subscription} from 'rxjs/Subscription'; import {ENTER, DOWN_ARROW, SPACE, UP_ARROW, HOME, END} from '../core/keyboard/keycodes'; import {MdOption} from '../core/option/option'; -import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; -import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler'; import {MdAutocomplete} from './autocomplete'; import {MdInputContainer} from '../input/input-container'; import {Observable} from 'rxjs/Observable'; @@ -65,10 +63,7 @@ describe('MdAutocomplete', () => { return {getContainerElement: () => overlayContainerElement}; }}, - {provide: Dir, useFactory: () => { - return {value: dir}; - }}, - {provide: ViewportRuler, useClass: FakeViewportRuler}, + {provide: Dir, useFactory: () => ({value: dir})}, {provide: ScrollDispatcher, useFactory: () => { return {scrolled: (delay: number, callback: () => any) => { return scrolledSubject.asObservable().subscribe(callback); @@ -929,8 +924,8 @@ describe('MdAutocomplete', () => { const panelTop = panel.getBoundingClientRect().top; // Panel is offset by 6px in styles so that the underline has room to display. - expect((inputBottom + 6).toFixed(1)) - .toEqual(panelTop.toFixed(1), `Expected panel top to match input bottom by default.`); + expect(Math.floor(inputBottom + 6)) + .toEqual(Math.floor(panelTop), `Expected panel top to match input bottom by default.`); expect(fixture.componentInstance.trigger.autocomplete.positionY) .toEqual('below', `Expected autocomplete positionY to default to below.`); }); @@ -952,7 +947,7 @@ describe('MdAutocomplete', () => { const panel = overlayContainerElement.querySelector('.mat-autocomplete-panel'); const panelTop = panel.getBoundingClientRect().top; - expect((inputBottom + 6).toFixed(1)).toEqual(panelTop.toFixed(1), + expect(Math.floor(inputBottom + 6)).toEqual(Math.floor(panelTop), 'Expected panel top to match input bottom after scrolling.'); document.body.removeChild(spacer); @@ -971,8 +966,8 @@ describe('MdAutocomplete', () => { const panelBottom = panel.getBoundingClientRect().bottom; // Panel is offset by 24px in styles so that the label has room to display. - expect((inputTop - 24).toFixed(1)) - .toEqual(panelBottom.toFixed(1), `Expected panel to fall back to above position.`); + expect(Math.floor(inputTop - 24)) + .toEqual(Math.floor(panelBottom), `Expected panel to fall back to above position.`); expect(fixture.componentInstance.trigger.autocomplete.positionY) .toEqual('above', `Expected autocomplete positionY to be "above" if panel won't fit.`); }); @@ -994,8 +989,8 @@ describe('MdAutocomplete', () => { const panelBottom = panel.getBoundingClientRect().bottom; // Panel is offset by 24px in styles so that the label has room to display. - expect((inputTop - 24).toFixed(1)) - .toEqual(panelBottom.toFixed(1), `Expected panel to stay aligned after filtering.`); + expect(Math.floor(inputTop - 24)) + .toEqual(Math.floor(panelBottom), `Expected panel to stay aligned after filtering.`); expect(fixture.componentInstance.trigger.autocomplete.positionY) .toEqual('above', `Expected autocomplete positionY to be "above" if panel won't fit.`); }); diff --git a/src/lib/core/overlay/_overlay.scss b/src/lib/core/overlay/_overlay.scss index 91d82ee81a12..ce6a99b5411a 100644 --- a/src/lib/core/overlay/_overlay.scss +++ b/src/lib/core/overlay/_overlay.scss @@ -9,8 +9,10 @@ // The container should be the size of the viewport. top: 0; left: 0; - height: 100%; - width: 100%; + + // Note: we prefer viewport units, because they aren't being offset by the global scrollbar. + height: 100vh; + width: 100vw; } // The overlay-container is an invisible element which contains all individual overlays. diff --git a/src/lib/core/overlay/position/connected-position-strategy.spec.ts b/src/lib/core/overlay/position/connected-position-strategy.spec.ts index fb0dcb66bb73..5cdd56bc3223 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.spec.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.spec.ts @@ -41,7 +41,6 @@ describe('ConnectedPositionStrategy', () => { let overlayContainerElement: HTMLElement; let strategy: ConnectedPositionStrategy; let fakeElementRef: ElementRef; - let fakeViewportRuler: FakeViewportRuler; let positionBuilder: OverlayPositionBuilder; let originRect: ClientRect; @@ -49,11 +48,9 @@ describe('ConnectedPositionStrategy', () => { let originCenterY: number; beforeEach(() => { - fakeViewportRuler = new FakeViewportRuler(); - // The origin and overlay elements need to be in the document body in order to have geometry. originElement = createPositionedBlockElement(); - overlayContainerElement = createFixedElement(); + overlayContainerElement = createOverlayContainer(); overlayElement = createPositionedBlockElement(); document.body.appendChild(originElement); document.body.appendChild(overlayContainerElement); @@ -148,8 +145,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.left).toBe(originRect.left); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left)); }); it('should reposition the overlay if it would go off the left of the screen', () => { @@ -172,18 +169,14 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originCenterY - (OVERLAY_HEIGHT / 2)); - expect(overlayRect.left).toBe(originRect.right); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originCenterY - (OVERLAY_HEIGHT / 2))); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.right)); }); it('should reposition the overlay if it would go off the bottom of the screen', () => { - // Use the fake viewport ruler because we don't know *exactly* how big the viewport is. - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); - originElement.style.top = '475px'; + originElement.style.bottom = '25px'; originElement.style.left = '200px'; originRect = originElement.getBoundingClientRect(); @@ -198,19 +191,15 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.bottom).toBe(originRect.top); - expect(overlayRect.right).toBe(originRect.right); + expect(Math.floor(overlayRect.bottom)).toBe(Math.floor(originRect.top)); + expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.right)); }); it('should reposition the overlay if it would go off the right of the screen', () => { - // Use the fake viewport ruler because we don't know *exactly* how big the viewport is. - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); originElement.style.top = '200px'; - originElement.style.left = '475px'; + originElement.style.right = '25px'; originRect = originElement.getBoundingClientRect(); strategy = positionBuilder.connectedTo( @@ -224,19 +213,16 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.right).toBe(originRect.left); + + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.left)); }); it('should recalculate and set the last position with recalculateLastPosition()', () => { - // Use the fake viewport ruler because we don't know *exactly* how big the viewport is. - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); // Push the trigger down so the overlay doesn't have room to open on the bottom. - originElement.style.top = '475px'; + originElement.style.bottom = '25px'; originRect = originElement.getBoundingClientRect(); strategy = positionBuilder.connectedTo( @@ -257,16 +243,12 @@ describe('ConnectedPositionStrategy', () => { strategy.recalculateLastPosition(); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.bottom).toBe(originRect.top, + expect(Math.floor(overlayRect.bottom)).toBe(Math.floor(originRect.top), 'Expected overlay to be re-aligned to the trigger in the previous position.'); }); it('should default to the initial position, if no positions fit in the viewport', () => { - // Use the fake viewport ruler because we don't know *exactly* how big the viewport is. - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); // Make the origin element taller than the viewport. originElement.style.height = '1000px'; @@ -283,7 +265,7 @@ describe('ConnectedPositionStrategy', () => { let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.bottom).toBe(originRect.top, + expect(Math.floor(overlayRect.bottom)).toBe(Math.floor(originRect.top), 'Expected overlay to be re-aligned to the trigger in the initial position.'); }); @@ -300,8 +282,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.right).toBe(originRect.right); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.right)); }); it('should position a panel with the x offset provided', () => { @@ -315,8 +297,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.top); - expect(overlayRect.left).toBe(originRect.left + 10); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left + 10)); }); it('should position a panel with the y offset provided', () => { @@ -330,20 +312,16 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.top + 50); - expect(overlayRect.left).toBe(originRect.left); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top + 50)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left)); }); }); it('should emit onPositionChange event when position changes', () => { - // force the overlay to open in a fallback position - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); originElement.style.top = '200px'; - originElement.style.left = '475px'; + originElement.style.right = '25px'; strategy = positionBuilder.connectedTo( fakeElementRef, @@ -363,14 +341,10 @@ describe('ConnectedPositionStrategy', () => { `Expected strategy to emit an instance of ConnectedOverlayPositionChange.`); it('should pick the fallback position that shows the largest area of the element', () => { - // Use the fake viewport ruler because we don't know *exactly* how big the viewport is. - fakeViewportRuler.fakeRect = { - top: 0, left: 0, width: 500, height: 500, right: 500, bottom: 500 - }; - positionBuilder = new OverlayPositionBuilder(fakeViewportRuler); + positionBuilder = new OverlayPositionBuilder(viewportRuler); originElement.style.top = '200px'; - originElement.style.left = '475px'; + originElement.style.right = '25px'; originRect = originElement.getBoundingClientRect(); strategy = positionBuilder.connectedTo( @@ -388,8 +362,8 @@ describe('ConnectedPositionStrategy', () => { let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.top); - expect(overlayRect.left).toBe(originRect.left); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left)); }); it('should position a panel properly when rtl', () => { @@ -430,8 +404,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.left).toBe(originRect.left); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left)); }); it('should position to the right, center aligned vertically', () => { @@ -443,8 +417,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originCenterY - (OVERLAY_HEIGHT / 2)); - expect(overlayRect.left).toBe(originRect.right); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originCenterY - (OVERLAY_HEIGHT / 2))); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.right)); }); it('should position to the left, below', () => { @@ -456,8 +430,9 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.right).toBe(originRect.left); + + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.left)); }); it('should position above, right aligned', () => { @@ -469,8 +444,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.bottom).toBe(originRect.top); - expect(overlayRect.right).toBe(originRect.right); + expect(Math.floor(overlayRect.bottom)).toBe(Math.floor(originRect.top)); + expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.right)); }); it('should position below, centered', () => { @@ -482,8 +457,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.bottom); - expect(overlayRect.left).toBe(originCenterX - (OVERLAY_WIDTH / 2)); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originCenterX - (OVERLAY_WIDTH / 2))); }); it('should center the overlay on the origin', () => { @@ -495,8 +470,8 @@ describe('ConnectedPositionStrategy', () => { strategy.apply(overlayElement); let overlayRect = overlayElement.getBoundingClientRect(); - expect(overlayRect.top).toBe(originRect.top); - expect(overlayRect.left).toBe(originRect.left); + expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top)); + expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left)); }); } }); @@ -513,7 +488,7 @@ describe('ConnectedPositionStrategy', () => { beforeEach(() => { // Set up the overlay - overlayContainerElement = createFixedElement(); + overlayContainerElement = createOverlayContainer(); overlayElement = createPositionedBlockElement(); document.body.appendChild(overlayContainerElement); overlayContainerElement.appendChild(overlayElement); @@ -609,8 +584,6 @@ describe('ConnectedPositionStrategy', () => { function createPositionedBlockElement() { let element = createBlockElement(); element.style.position = 'absolute'; - element.style.top = '0'; - element.style.left = '0'; return element; } @@ -624,15 +597,10 @@ function createBlockElement() { return element; } -/** Creates an position: fixed element that spans the screen size. */ -function createFixedElement() { +/** Creates the wrapper for all of the overlays. */ +function createOverlayContainer() { let element = document.createElement('div'); - element.style.position = 'fixed'; - element.style.top = '0'; - element.style.left = '0'; - element.style.width = `100%`; - element.style.height = `100%`; - element.style.zIndex = '100'; + element.classList.add('cdk-overlay-container'); return element; } @@ -648,23 +616,7 @@ function createOverflowContainerElement() { } -/** Fake implementation of ViewportRuler that just returns the previously given ClientRect. */ -class FakeViewportRuler implements ViewportRuler { - fakeRect: ClientRect = {left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014}; - fakeScrollPos: {top: number, left: number} = {top: 0, left: 0}; - - getViewportRect() { - return this.fakeRect; - } - - getViewportScrollPosition(documentRect?: ClientRect): {top: number; left: number} { - return this.fakeScrollPos; - } -} - - /** Fake implementation of ElementRef that is just a simple container for nativeElement. */ class FakeElementRef implements ElementRef { - constructor(public nativeElement: HTMLElement) { - } + constructor(public nativeElement: HTMLElement) { } } diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 2eda42461e93..2f97d4c7feae 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -108,6 +108,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { // Fallback point if none of the fallbacks fit into the viewport. let fallbackPoint: OverlayPoint = null; + let fallbackPosition: ConnectionPositionPair = null; // We want to place the overlay in the first of the preferred positions such that the // overlay fits on-screen. @@ -116,10 +117,11 @@ export class ConnectedPositionStrategy implements PositionStrategy { // (top, left) coordinate for the overlay at `pos`. let originPoint = this._getOriginConnectionPoint(originRect, pos); let overlayPoint = this._getOverlayPoint(originPoint, overlayRect, viewportRect, pos); + let overlayDimensions = this._getCSSDimensions(overlayRect, overlayPoint, pos); // If the overlay in the calculated position fits on-screen, put it there and we're done. if (overlayPoint.fitsInViewport) { - this._setElementPosition(element, overlayPoint); + this._setElementPosition(element, overlayDimensions); // Save the last connected position in case the position needs to be re-calculated. this._lastConnectedPosition = pos; @@ -132,12 +134,14 @@ export class ConnectedPositionStrategy implements PositionStrategy { return Promise.resolve(null); } else if (!fallbackPoint || fallbackPoint.visibleArea < overlayPoint.visibleArea) { fallbackPoint = overlayPoint; + fallbackPosition = pos; } } // If none of the preferred positions were in the viewport, take the one // with the largest visible area. - this._setElementPosition(element, fallbackPoint); + let fallbackDimensions = this._getCSSDimensions(overlayRect, fallbackPoint, fallbackPosition); + this._setElementPosition(element, fallbackDimensions); return Promise.resolve(null); } @@ -155,7 +159,8 @@ export class ConnectedPositionStrategy implements PositionStrategy { let originPoint = this._getOriginConnectionPoint(originRect, lastPosition); let overlayPoint = this._getOverlayPoint(originPoint, overlayRect, viewportRect, lastPosition); - this._setElementPosition(this._pane, overlayPoint); + let overlayPosition = this._getCSSDimensions(overlayRect, overlayPoint, lastPosition); + this._setElementPosition(this._pane, overlayPosition); } /** @@ -299,6 +304,39 @@ export class ConnectedPositionStrategy implements PositionStrategy { return {x, y, fitsInViewport, visibleArea}; } + /** + * Determines which CSS properties to use when positioning the overlay, + * depending on the direction the element would expand in, if extra content + * was added. + */ + private _getCSSDimensions(overlayRect: ClientRect, overlayPoint: Point, + pos: ConnectionPositionPair): CSSDimensionPair { + + const viewport = this._viewportRuler.getViewportRect(); + const x: CSSDimension = { property: null, value: null }; + const y: CSSDimension = { property: pos.overlayY === 'bottom' ? 'bottom' : 'top', value: null }; + + if (this._dir === 'rtl') { + x.property = pos.overlayX === 'end' ? 'left' : 'right'; + } else { + x.property = pos.overlayX === 'end' ? 'right' : 'left'; + } + + if (x.property === 'left') { + x.value = overlayPoint.x; + } else { + x.value = viewport.width - (overlayPoint.x + overlayRect.width); + } + + if (y.property === 'top') { + y.value = overlayPoint.y; + } else { + y.value = viewport.height - (overlayPoint.y + overlayRect.height); + } + + return {x, y}; + } + /** * Gets the view properties of the trigger and overlay, including whether they are clipped * or completely outside the view of any of the strategy's scrollables. @@ -346,14 +384,11 @@ export class ConnectedPositionStrategy implements PositionStrategy { }); } - /** - * Physically positions the overlay element to the given coordinate. - * @param element - * @param overlayPoint - */ - private _setElementPosition(element: HTMLElement, overlayPoint: Point) { - element.style.left = overlayPoint.x + 'px'; - element.style.top = overlayPoint.y + 'px'; + /** Physically positions the overlay element to the given coordinate. */ + private _setElementPosition(element: HTMLElement, dimensions: CSSDimensionPair) { + ['top', 'bottom', 'left', 'right'].forEach(prop => element.style[prop] = null); + element.style[dimensions.x.property] = dimensions.x.value + 'px'; + element.style[dimensions.y.property] = dimensions.y.value + 'px'; } /** Returns the bounding positions of the provided element with respect to the viewport. */ @@ -392,3 +427,15 @@ interface OverlayPoint extends Point { visibleArea?: number; fitsInViewport?: boolean; } + +/** Key-value pair, representing a CSS dimension. */ +interface CSSDimension { + property: 'top' | 'bottom' | 'left' | 'right'; + value: number; +} + +/** A combination of CSS dimensions for the x and y axis. */ +interface CSSDimensionPair { + x: CSSDimension; + y: CSSDimension; +} diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 849e84a8aa76..551455d7a079 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -34,9 +34,7 @@ describe('MdMenu', () => { providers: [ {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); - overlayContainerElement.style.position = 'fixed'; - overlayContainerElement.style.top = '0'; - overlayContainerElement.style.left = '0'; + overlayContainerElement.classList.add('cdk-overlay-container'); document.body.appendChild(overlayContainerElement); // remove body padding to keep consistent cross-browser @@ -46,8 +44,7 @@ describe('MdMenu', () => { }}, {provide: Dir, useFactory: () => { return {value: dir}; - }}, - {provide: ViewportRuler, useClass: FakeViewportRuler} + }} ] }); @@ -115,7 +112,7 @@ describe('MdMenu', () => { const trigger = fixture.componentInstance.triggerEl.nativeElement; // Push trigger to the bottom edge of viewport,so it has space to open "above" - trigger.style.position = 'relative'; + trigger.style.position = 'fixed'; trigger.style.top = '600px'; // Push trigger to the right, so it has space to open "before" @@ -148,8 +145,9 @@ describe('MdMenu', () => { // Push trigger to the right side of viewport, so it doesn't have space to open // in its default "after" position on the right side. - trigger.style.position = 'relative'; - trigger.style.left = '950px'; + trigger.style.position = 'fixed'; + trigger.style.right = '-50px'; + trigger.style.top = '200px'; fixture.componentInstance.trigger.openMenu(); fixture.detectChanges(); @@ -160,13 +158,13 @@ describe('MdMenu', () => { // In "before" position, the right sides of the overlay and the origin are aligned. // To find the overlay left, subtract the menu width from the origin's right side. const expectedLeft = triggerRect.right - overlayRect.width; - expect(Math.round(overlayRect.left)) - .toBe(Math.round(expectedLeft), + expect(Math.floor(overlayRect.left)) + .toBe(Math.floor(expectedLeft), `Expected menu to open in "before" position if "after" position wouldn't fit.`); // The y-position of the overlay should be unaffected, as it can already fit vertically - expect(Math.round(overlayRect.top)) - .toBe(Math.round(triggerRect.top), + expect(Math.floor(overlayRect.top)) + .toBe(Math.floor(triggerRect.top), `Expected menu top position to be unchanged if it can fit in the viewport.`); }); @@ -177,7 +175,7 @@ describe('MdMenu', () => { // Push trigger to the bottom part of viewport, so it doesn't have space to open // in its default "below" position below the trigger. - trigger.style.position = 'relative'; + trigger.style.position = 'fixed'; trigger.style.top = '600px'; fixture.componentInstance.trigger.openMenu(); @@ -189,13 +187,13 @@ describe('MdMenu', () => { // In "above" position, the bottom edges of the overlay and the origin are aligned. // To find the overlay top, subtract the menu height from the origin's bottom edge. const expectedTop = triggerRect.bottom - overlayRect.height; - expect(Math.round(overlayRect.top)) - .toBe(Math.round(expectedTop), + expect(Math.floor(overlayRect.top)) + .toBe(Math.floor(expectedTop), `Expected menu to open in "above" position if "below" position wouldn't fit.`); // The x-position of the overlay should be unaffected, as it can already fit horizontally - expect(Math.round(overlayRect.left)) - .toBe(Math.round(triggerRect.left), + expect(Math.floor(overlayRect.left)) + .toBe(Math.floor(triggerRect.left), `Expected menu x position to be unchanged if it can fit in the viewport.`); }); @@ -206,8 +204,8 @@ describe('MdMenu', () => { // push trigger to the bottom, right part of viewport, so it doesn't have space to open // in its default "after below" position. - trigger.style.position = 'relative'; - trigger.style.left = '950px'; + trigger.style.position = 'fixed'; + trigger.style.right = '-50px'; trigger.style.top = '600px'; fixture.componentInstance.trigger.openMenu(); @@ -219,12 +217,12 @@ describe('MdMenu', () => { const expectedLeft = triggerRect.right - overlayRect.width; const expectedTop = triggerRect.bottom - overlayRect.height; - expect(Math.round(overlayRect.left)) - .toBe(Math.round(expectedLeft), + expect(Math.floor(overlayRect.left)) + .toBe(Math.floor(expectedLeft), `Expected menu to open in "before" position if "after" position wouldn't fit.`); - expect(Math.round(overlayRect.top)) - .toBe(Math.round(expectedTop), + expect(Math.floor(overlayRect.top)) + .toBe(Math.floor(expectedTop), `Expected menu to open in "above" position if "below" position wouldn't fit.`); }); @@ -241,14 +239,14 @@ describe('MdMenu', () => { // As designated "before" position won't fit on screen, the menu should fall back // to "after" mode, where the left sides of the overlay and trigger are aligned. - expect(Math.round(overlayRect.left)) - .toBe(Math.round(triggerRect.left), + expect(Math.floor(overlayRect.left)) + .toBe(Math.floor(triggerRect.left), `Expected menu to open in "after" position if "before" position wouldn't fit.`); // As designated "above" position won't fit on screen, the menu should fall back // to "below" mode, where the top edges of the overlay and trigger are aligned. - expect(Math.round(overlayRect.top)) - .toBe(Math.round(triggerRect.top), + expect(Math.floor(overlayRect.top)) + .toBe(Math.floor(triggerRect.top), `Expected menu to open in "below" position if "above" position wouldn't fit.`); }); @@ -315,8 +313,8 @@ describe('MdMenu', () => { subject.openMenu(); // Since the menu is overlaying the trigger, the overlay top should be the trigger top. - expect(Math.round(subject.overlayRect.top)) - .toBe(Math.round(subject.triggerRect.top), + expect(Math.floor(subject.overlayRect.top)) + .toBe(Math.floor(subject.triggerRect.top), `Expected menu to open in default "below" position.`); }); }); @@ -330,20 +328,20 @@ describe('MdMenu', () => { subject.openMenu(); // Since the menu is below the trigger, the overlay top should be the trigger bottom. - expect(Math.round(subject.overlayRect.top)) - .toBe(Math.round(subject.triggerRect.bottom), + expect(Math.floor(subject.overlayRect.top)) + .toBe(Math.floor(subject.triggerRect.bottom), `Expected menu to open directly below the trigger.`); }); it('supports above position fall back', () => { // Push trigger to the bottom part of viewport, so it doesn't have space to open // in its default "below" position below the trigger. - subject.updateTriggerStyle({position: 'relative', top: '650px'}); + subject.updateTriggerStyle({position: 'fixed', bottom: '0'}); subject.openMenu(); // Since the menu is above the trigger, the overlay bottom should be the trigger top. - expect(Math.round(subject.overlayRect.bottom)) - .toBe(Math.round(subject.triggerRect.top), + expect(Math.floor(subject.overlayRect.bottom)) + .toBe(Math.floor(subject.triggerRect.top), `Expected menu to open in "above" position if "below" position wouldn't fit.`); }); @@ -500,15 +498,3 @@ class CustomMenuPanel implements MdMenuPanel { class CustomMenu { @ViewChild(MdMenuTrigger) trigger: MdMenuTrigger; } - -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.spec.ts b/src/lib/select/select.spec.ts index 974176117de8..2c9c6a7eb973 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -1,4 +1,4 @@ -import {TestBed, async, ComponentFixture, fakeAsync, tick} from '@angular/core/testing'; +import {TestBed, async, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import { Component, @@ -28,23 +28,11 @@ import {TAB} from '../core/keyboard/keycodes'; import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; -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 scrolledSubject = new Subject(); - let fakeViewportRuler = new FakeViewportRuler(); + let viewportRuler: ViewportRuler; beforeEach(async(() => { TestBed.configureTestingModule({ @@ -83,10 +71,7 @@ describe('MdSelect', () => { return {getContainerElement: () => overlayContainerElement}; }}, - {provide: ViewportRuler, useValue: fakeViewportRuler}, - {provide: Dir, useFactory: () => { - return dir = { value: 'ltr' }; - }}, + {provide: Dir, useFactory: () => dir = { value: 'ltr' }}, {provide: ScrollDispatcher, useFactory: () => { return {scrolled: (delay: number, callback: () => any) => { return scrolledSubject.asObservable().subscribe(callback); @@ -98,6 +83,10 @@ describe('MdSelect', () => { TestBed.compileComponents(); })); + beforeEach(inject([ViewportRuler], (_ruler: ViewportRuler) => { + viewportRuler = _ruler; + })); + afterEach(() => { document.body.removeChild(overlayContainerElement); }); @@ -755,12 +744,12 @@ describe('MdSelect', () => { // The option text should align with the trigger text. Because each option is 18px // larger in height than the trigger, the option needs to be adjusted up 9 pixels. - expect(optionTop.toFixed(2)) - .toEqual((triggerTop - 9).toFixed(2), `Expected trigger to align with option ${index}.`); + expect(Math.floor(optionTop)) + .toEqual(Math.floor(triggerTop - 9), `Expected trigger to align with option ${index}.`); // For the animation to start at the option's center, its origin must be the distance // from the top of the overlay to the option top + half the option height (48/2 = 24). - const expectedOrigin = optionTop - overlayTop + 24; + const expectedOrigin = Math.floor(optionTop - overlayTop + 24); expect(fixture.componentInstance.select._transformOrigin) .toContain(`${expectedOrigin}px`, `Expected panel animation to originate in the center of option ${index}.`); @@ -920,8 +909,8 @@ describe('MdSelect', () => { // Expect no scroll to be attempted expect(scrollContainer.scrollTop).toEqual(0, `Expected panel not to be scrolled.`); - expect(overlayBottom.toFixed(2)) - .toEqual(triggerBottom.toFixed(2), + expect(Math.floor(overlayBottom)) + .toEqual(Math.floor(triggerBottom), `Expected trigger bottom to align with overlay bottom.`); expect(fixture.componentInstance.select._transformOrigin) @@ -947,8 +936,8 @@ describe('MdSelect', () => { // Expect scroll to remain at the max scroll position expect(scrollContainer.scrollTop).toEqual(128, `Expected panel to be at max scroll.`); - expect(overlayTop.toFixed(2)) - .toEqual(triggerTop.toFixed(2), `Expected trigger top to align with overlay top.`); + expect(Math.floor(overlayTop)) + .toEqual(Math.floor(triggerTop), `Expected trigger top to align with overlay top.`); expect(fixture.componentInstance.select._transformOrigin) .toContain(`top`, `Expected panel animation to originate at the top.`); @@ -994,7 +983,7 @@ describe('MdSelect', () => { tick(400); fixture.detectChanges(); - const viewportRect = fakeViewportRuler.getViewportRect().right; + const viewportRect = viewportRuler.getViewportRect().right; const panelRight = document.querySelector('.mat-select-panel') .getBoundingClientRect().right; @@ -1009,7 +998,7 @@ describe('MdSelect', () => { tick(400); fixture.detectChanges(); - const viewportRect = fakeViewportRuler.getViewportRect().right; + const viewportRect = viewportRuler.getViewportRect().right; const panelRight = document.querySelector('.mat-select-panel') .getBoundingClientRect().right; @@ -1126,8 +1115,8 @@ describe('MdSelect', () => { const triggerBottom = trigger.getBoundingClientRect().bottom; const overlayBottom = overlayPane.getBoundingClientRect().bottom; - expect(overlayBottom.toFixed(2)) - .toEqual(triggerBottom.toFixed(2), + expect(Math.floor(overlayBottom)) + .toEqual(Math.floor(triggerBottom), `Expected trigger bottom to align with overlay bottom.`); }); @@ -1150,8 +1139,8 @@ describe('MdSelect', () => { const triggerTop = trigger.getBoundingClientRect().top; const overlayTop = overlayPane.getBoundingClientRect().top; - expect(overlayTop.toFixed(2)) - .toEqual(triggerTop.toFixed(2), `Expected trigger top to align with overlay top.`); + expect(Math.floor(overlayTop)) + .toEqual(Math.floor(triggerTop), `Expected trigger top to align with overlay top.`); }); }); @@ -1173,7 +1162,7 @@ describe('MdSelect', () => { // 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), + expect(Math.floor(firstOptionLeft)).toEqual(Math.floor(triggerLeft - 16), `Expected trigger to align with the selected option on the x-axis in LTR.`); })); @@ -1191,8 +1180,8 @@ describe('MdSelect', () => { // 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), + expect(Math.floor(firstOptionRight)) + .toEqual(Math.floor(triggerRight + 16), `Expected trigger to align with the selected option on the x-axis in RTL.`); })); }); @@ -1220,8 +1209,8 @@ describe('MdSelect', () => { document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().left; // 48px accounts for the checkbox size, margin and the panel's padding. - expect(firstOptionLeft.toFixed(2)) - .toEqual((triggerLeft - 48).toFixed(2), + expect(Math.floor(firstOptionLeft)) + .toEqual(Math.floor(triggerLeft - 48), `Expected trigger label to align along x-axis, accounting for the checkbox.`); }); })); @@ -1237,8 +1226,8 @@ describe('MdSelect', () => { 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), + expect(Math.floor(firstOptionRight)) + .toEqual(Math.floor(triggerRight + 48), `Expected trigger label to align along x-axis, accounting for the checkbox.`); })); });