Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(menu): update to use overlay backdrop #1534

Merged
merged 1 commit into from
Oct 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions e2e/components/menu/menu.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 0 additions & 10 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
23 changes: 22 additions & 1 deletion src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -70,13 +72,15 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
if (!this._menuOpen) {
this._createOverlay();
this._overlayRef.attach(this._portal);
this._subscribeToBackdrop();
this._initMenu();
}
}

closeMenu(): void {
if (this._overlayRef) {
this._overlayRef.detach();
this._backdropSubscription.unsubscribe();
this._resetMenu();
}
}
Expand All @@ -85,13 +89,29 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
if (this._overlayRef) {
this._overlayRef.dispose();
this._overlayRef = null;

if (this._backdropSubscription) {
this._backdropSubscription.unsubscribe();
}
}
}

focus() {
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.
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/menu/menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
<ng-content></ng-content>
</div>
</template>
<div class="md-menu-click-catcher" *ngIf="_showClickCatcher" (click)="_emitCloseEvent()"></div>

6 changes: 1 addition & 5 deletions src/lib/menu/menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,4 @@ $md-menu-vertical-padding: 8px !default;

button[md-menu-item] {
width: 100%;
}

.md-menu-click-catcher {
@include md-fullscreen();
}
}
32 changes: 25 additions & 7 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
@@ -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 = <HTMLElement>overlayContainerElement.querySelector('.md-overlay-backdrop');
backdrop.click();
fixture.detectChanges();

expect(overlayContainerElement.textContent).toBe('');
});

});

@Component({
template: `
<button [md-menu-trigger-for]="menu">Toggle menu</button>
<md-menu #menu="mdMenu">
Content
<button md-menu-item> Content </button>
</md-menu>
`
})
Expand Down