From f3d26c2d132c20f4ae4806637867e45ef7ed233b Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Fri, 21 Oct 2016 15:22:55 -0700 Subject: [PATCH] fix(menu): update to use overlay backdrop --- e2e/components/menu/menu.e2e.ts | 6 ------ src/lib/menu/menu-directive.ts | 10 ---------- src/lib/menu/menu-trigger.ts | 23 ++++++++++++++++++++++- src/lib/menu/menu.html | 2 +- src/lib/menu/menu.scss | 6 +----- src/lib/menu/menu.spec.ts | 32 +++++++++++++++++++++++++------- 6 files changed, 49 insertions(+), 30 deletions(-) diff --git a/e2e/components/menu/menu.e2e.ts b/e2e/components/menu/menu.e2e.ts index dcb5202925e2..bdda6616b2ac 100644 --- a/e2e/components/menu/menu.e2e.ts +++ b/e2e/components/menu/menu.e2e.ts @@ -15,12 +15,6 @@ describe('menu', () => { expect(page.menu().getText()).toEqual("One\nTwo\nThree\nFour"); }); - it('should close menu when area outside menu is clicked', () => { - page.trigger().click(); - page.body().click(); - page.expectMenuPresent(false); - }); - it('should close menu when menu item is clicked', () => { page.trigger().click(); page.items(0).click(); diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts index 51c038f83383..2a3eeaa36b7c 100644 --- a/src/lib/menu/menu-directive.ts +++ b/src/lib/menu/menu-directive.ts @@ -27,7 +27,6 @@ import {UP_ARROW, DOWN_ARROW, TAB} from '../core'; exportAs: 'mdMenu' }) export class MdMenu { - _showClickCatcher: boolean = false; private _focusedItemIndex: number = 0; // config object to be passed into the menu's ngClass @@ -61,15 +60,6 @@ export class MdMenu { @Output() close = new EventEmitter; - /** - * This function toggles the display of the menu's click catcher element. - * This element covers the viewport when the menu is open to detect clicks outside the menu. - * TODO: internal - */ - _setClickCatcher(bool: boolean): void { - this._showClickCatcher = bool; - } - /** * Focus the first item in the menu. This method is used by the menu trigger * to focus the first item when the menu is opened by the ENTER key. diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index a77346ad2560..474259790bfe 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -23,6 +23,7 @@ import { HorizontalConnectionPos, VerticalConnectionPos } from '../core'; +import { Subscription } from 'rxjs/Subscription'; /** * This directive is intended to be used in conjunction with an md-menu tag. It is @@ -40,6 +41,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { private _portal: TemplatePortal; private _overlayRef: OverlayRef; private _menuOpen: boolean = false; + private _backdropSubscription: Subscription; // tracking input type is necessary so it's possible to only auto-focus // the first item of the list when the menu is opened via the keyboard @@ -70,6 +72,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { if (!this._menuOpen) { this._createOverlay(); this._overlayRef.attach(this._portal); + this._subscribeToBackdrop(); this._initMenu(); } } @@ -77,6 +80,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { closeMenu(): void { if (this._overlayRef) { this._overlayRef.detach(); + this._backdropSubscription.unsubscribe(); this._resetMenu(); } } @@ -85,6 +89,10 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { if (this._overlayRef) { this._overlayRef.dispose(); this._overlayRef = null; + + if (this._backdropSubscription) { + this._backdropSubscription.unsubscribe(); + } } } @@ -92,6 +100,18 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { this._renderer.invokeElementMethod(this._element.nativeElement, 'focus'); } + /** + * This method ensures that the menu closes when the overlay backdrop is clicked. + * We do not use first() here because doing so would not catch clicks from within + * the menu, and it would fail to unsubscribe properly. Instead, we unsubscribe + * explicitly when the menu is closed or destroyed. + */ + private _subscribeToBackdrop(): void { + this._backdropSubscription = this._overlayRef.backdropClick().subscribe(() => { + this.closeMenu(); + }); + } + /** * This method sets the menu state to open and focuses the first item if * the menu was opened via the keyboard. @@ -120,7 +140,6 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { // set state rather than toggle to support triggers sharing a menu private _setIsMenuOpen(isOpen: boolean): void { this._menuOpen = isOpen; - this.menu._setClickCatcher(isOpen); this._menuOpen ? this.onMenuOpen.emit(null) : this.onMenuClose.emit(null); } @@ -152,6 +171,8 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { private _getOverlayConfig(): OverlayState { const overlayState = new OverlayState(); overlayState.positionStrategy = this._getPosition(); + overlayState.hasBackdrop = true; + overlayState.backdropClass = 'md-overlay-transparent-backdrop'; return overlayState; } diff --git a/src/lib/menu/menu.html b/src/lib/menu/menu.html index bb6d522a79d7..58721749878e 100644 --- a/src/lib/menu/menu.html +++ b/src/lib/menu/menu.html @@ -4,4 +4,4 @@ -
+ diff --git a/src/lib/menu/menu.scss b/src/lib/menu/menu.scss index c939bd12a0dc..fafa964c6c22 100644 --- a/src/lib/menu/menu.scss +++ b/src/lib/menu/menu.scss @@ -52,8 +52,4 @@ $md-menu-vertical-padding: 8px !default; button[md-menu-item] { width: 100%; -} - -.md-menu-click-catcher { - @include md-fullscreen(); -} +} \ No newline at end of file diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index baa0bb8accaa..f702bd81b75a 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -1,40 +1,58 @@ import {TestBed, async} from '@angular/core/testing'; import {Component, ViewChild} from '@angular/core'; -import {By} from '@angular/platform-browser'; import {MdMenuModule, MdMenuTrigger} from './menu'; +import {OverlayContainer} from '../core/overlay/overlay-container'; describe('MdMenu', () => { + let overlayContainerElement: HTMLElement; beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdMenuModule.forRoot()], declarations: [SimpleMenu], + providers: [ + {provide: OverlayContainer, useFactory: () => { + overlayContainerElement = document.createElement('div'); + return {getContainerElement: () => overlayContainerElement}; + }} + ] }); TestBed.compileComponents(); })); it('should open the menu as an idempotent operation', () => { - let fixture = TestBed.createComponent(SimpleMenu); + const fixture = TestBed.createComponent(SimpleMenu); fixture.detectChanges(); - let menu = fixture.debugElement.query(By.css('.md-menu-panel')); - expect(menu).toBe(null); + expect(overlayContainerElement.textContent).toBe(''); expect(() => { fixture.componentInstance.trigger.openMenu(); fixture.componentInstance.trigger.openMenu(); - menu = fixture.debugElement.query(By.css('.md-menu-panel')); - expect(menu.nativeElement.innerHTML.trim()).toEqual('Content'); + expect(overlayContainerElement.textContent.trim()).toBe('Content'); }).not.toThrowError(); }); + + it('should close the menu when a click occurs outside the menu', () => { + const fixture = TestBed.createComponent(SimpleMenu); + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + + const backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop'); + backdrop.click(); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent).toBe(''); + }); + }); @Component({ template: ` - Content + ` })