From 1ce8d3ea890f1ade58ce779b92bac7933676d343 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 7 Nov 2016 12:30:52 -0800 Subject: [PATCH] fix(menu): reposition menu if it would open off screen --- src/lib/menu/menu-trigger.ts | 39 ++++++--- src/lib/menu/menu.spec.ts | 149 +++++++++++++++++++++++++++++++++-- 2 files changed, 172 insertions(+), 16 deletions(-) diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index e6048e2fef38..68411770c1c5 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -20,7 +20,9 @@ import { TemplatePortal, ConnectedPositionStrategy, HorizontalConnectionPos, - VerticalConnectionPos + VerticalConnectionPos, + OriginConnectionPosition, + OverlayConnectionPosition } from '../core'; import { Subscription } from 'rxjs/Subscription'; @@ -181,14 +183,33 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { * @returns ConnectedPositionStrategy */ private _getPosition(): ConnectedPositionStrategy { - const positionX: HorizontalConnectionPos = this.menu.positionX === 'before' ? 'end' : 'start'; - const positionY: VerticalConnectionPos = this.menu.positionY === 'above' ? 'bottom' : 'top'; - - return this._overlay.position().connectedTo( - this._element, - {originX: positionX, originY: positionY}, - {overlayX: positionX, overlayY: positionY} - ); + const [posX, fallbackX]: HorizontalConnectionPos[] = + this.menu.positionX === 'before' ? ['end', 'start'] : ['start', 'end']; + + const [posY, fallbackY]: VerticalConnectionPos[] = + this.menu.positionY === 'above' ? ['bottom', 'top'] : ['top', 'bottom']; + + return this._overlay.position() + .connectedTo(this._element, this._originPos(posX, posY), this._overlayPos(posX, posY)) + .withFallbackPosition( + this._originPos(fallbackX, posY), this._overlayPos(fallbackX, posY)) + .withFallbackPosition( + this._originPos(posX, fallbackY), this._overlayPos(posX, fallbackY)) + .withFallbackPosition( + this._originPos(fallbackX, fallbackY), this._overlayPos(fallbackX, fallbackY)); + } + + + /** Converts the designated point into an OriginConnectionPosition. */ + private _originPos(x: HorizontalConnectionPos, + y: VerticalConnectionPos): OriginConnectionPosition { + return {originX: x, originY: y} as OriginConnectionPosition; + } + + /** Converts the designated point into an OverlayConnectionPosition. */ + private _overlayPos(x: HorizontalConnectionPos, + y: VerticalConnectionPos): OverlayConnectionPosition { + return {overlayX: x, overlayY: y} as OverlayConnectionPosition; } // TODO: internal diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 70ce391b6cb8..e193e6967819 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -2,6 +2,7 @@ import {TestBed, async} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import { Component, + ElementRef, EventEmitter, Output, TemplateRef, @@ -15,6 +16,7 @@ import { MenuPositionY } from './menu'; import {OverlayContainer} from '../core/overlay/overlay-container'; +import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; describe('MdMenu', () => { let overlayContainerElement: HTMLElement; @@ -26,14 +28,23 @@ describe('MdMenu', () => { providers: [ {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); + overlayContainerElement.style.position = 'fixed'; + overlayContainerElement.style.top = '0'; + overlayContainerElement.style.left = '0'; + document.body.appendChild(overlayContainerElement); return {getContainerElement: () => overlayContainerElement}; - }} + }}, + {provide: ViewportRuler, useClass: FakeViewportRuler} ] }); TestBed.compileComponents(); })); + afterEach(() => { + document.body.removeChild(overlayContainerElement); + }); + it('should open the menu as an idempotent operation', () => { const fixture = TestBed.createComponent(SimpleMenu); fixture.detectChanges(); @@ -42,8 +53,8 @@ describe('MdMenu', () => { fixture.componentInstance.trigger.openMenu(); fixture.componentInstance.trigger.openMenu(); - expect(overlayContainerElement.textContent).toContain('Simple Content'); - expect(overlayContainerElement.textContent).toContain('Disabled Content'); + expect(overlayContainerElement.textContent).toContain('Item'); + expect(overlayContainerElement.textContent).toContain('Disabled'); }).not.toThrowError(); }); @@ -110,6 +121,117 @@ describe('MdMenu', () => { expect(panel.classList).not.toContain('md-menu-below'); }); + describe('fallback positions', () => { + + it('should fall back to "before" mode if "after" mode would not fit on screen', () => { + const fixture = TestBed.createComponent(SimpleMenu); + fixture.detectChanges(); + const trigger = fixture.componentInstance.triggerEl.nativeElement; + + // 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.marginLeft = '900px'; + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const triggerRect = trigger.getBoundingClientRect(); + + // In "before" position, the right sides of the overlay and the origin are aligned. + // To find the overlay left, subtract the menu width (112) from the origin's right side. + const expectedLeft = triggerRect.right - 112; + expect(overlayPane.getBoundingClientRect().left) + .toEqual(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(overlayPane.getBoundingClientRect().top) + .toEqual(triggerRect.top, + `Expected menu top position to be unchanged if it can fit in the viewport.`); + }); + + it('should fall back to "above" mode if "below" mode would not fit on screen', () => { + const fixture = TestBed.createComponent(SimpleMenu); + fixture.detectChanges(); + const trigger = fixture.componentInstance.triggerEl.nativeElement; + + // 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.marginTop = '600px'; + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const triggerRect = trigger.getBoundingClientRect(); + + // 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. + // Menu height = 48 per item * 2 + 16px padding = 112px + const expectedTop = triggerRect.bottom - 112; + expect(overlayPane.getBoundingClientRect().top) + .toEqual(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(overlayPane.getBoundingClientRect().left) + .toEqual(triggerRect.left, + `Expected menu x position to be unchanged if it can fit in the viewport.`); + }); + + it('should re-position menu on both axes if both defaults would not fit', () => { + const fixture = TestBed.createComponent(SimpleMenu); + fixture.detectChanges(); + const trigger = fixture.componentInstance.triggerEl.nativeElement; + + // 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.marginLeft = '900px'; + trigger.style.marginTop = '600px'; + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const triggerRect = trigger.getBoundingClientRect(); + + const expectedTop = triggerRect.bottom - 112; + const expectedLeft = triggerRect.right - 112; + + expect(overlayPane.getBoundingClientRect().left) + .toEqual(expectedLeft, + `Expected menu to open in "before" position if "after" position wouldn't fit.`); + + expect(overlayPane.getBoundingClientRect().top) + .toEqual(expectedTop, + `Expected menu to open in "above" position if "below" position wouldn't fit.`); + }); + + it('should re-position a menu with custom position set', () => { + const fixture = TestBed.createComponent(PositionedMenu); + fixture.detectChanges(); + const trigger = fixture.componentInstance.triggerEl.nativeElement; + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const triggerRect = trigger.getBoundingClientRect(); + + // 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(overlayPane.getBoundingClientRect().left) + .toEqual(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(overlayPane.getBoundingClientRect().top) + .toEqual(triggerRect.top, + `Expected menu to open in "below" position if "above" position wouldn't fit.`); + }); + + }); + + + }); describe('animations', () => { @@ -142,20 +264,21 @@ describe('MdMenu', () => { @Component({ template: ` - + - - + + ` }) class SimpleMenu { @ViewChild(MdMenuTrigger) trigger: MdMenuTrigger; + @ViewChild('triggerEl') triggerEl: ElementRef; } @Component({ template: ` - + @@ -163,6 +286,7 @@ class SimpleMenu { }) class PositionedMenu { @ViewChild(MdMenuTrigger) trigger: MdMenuTrigger; + @ViewChild('triggerEl') triggerEl: ElementRef; } @@ -195,3 +319,14 @@ 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}; + } +} \ No newline at end of file