Skip to content

Commit 29ad153

Browse files
committed
addressed comments
1 parent 0ee8dbb commit 29ad153

File tree

7 files changed

+42
-25
lines changed

7 files changed

+42
-25
lines changed

src/components/menu/menu-item.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Directive, Input, HostBinding, HostListener} from '@angular/core';
1+
import {Directive, Input, HostBinding} from '@angular/core';
22

33
/**
44
* This directive is intended to be used inside an md-menu tag.
@@ -16,7 +16,10 @@ export class MdMenuItem {}
1616
*/
1717
@Directive({
1818
selector: 'a[md-menu-item]',
19-
host: {'role': 'menuitem'}
19+
host: {
20+
'role': 'menuitem',
21+
'(click)': 'checkDisabled($event)'
22+
}
2023
})
2124
export class MdMenuAnchor {
2225
_disabled: boolean;
@@ -33,16 +36,15 @@ export class MdMenuAnchor {
3336

3437
@HostBinding('attr.aria-disabled')
3538
get isAriaDisabled(): string {
36-
return this.disabled ? 'true' : 'false';
39+
return String(this.disabled);
3740
}
3841

3942
@HostBinding('tabIndex')
4043
get tabIndex(): number {
4144
return this.disabled ? -1 : 0;
4245
}
4346

44-
@HostListener('click', ['$event'])
45-
checkDisabled(event: any) {
47+
checkDisabled(event: Event) {
4648
if (this.disabled) {
4749
event.preventDefault();
4850
event.stopPropagation();

src/components/menu/menu-trigger.ts

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ import {
3535
})
3636
export class MdMenuTrigger implements AfterViewInit, OnDestroy {
3737
private _portal: TemplatePortal;
38-
private _overlay: OverlayRef;
38+
private _overlayRef: OverlayRef;
3939
menuOpen: boolean = false;
4040

4141
@Input('md-menu-trigger-for') menu: MdMenu;
4242
@Output() onMenuOpen = new EventEmitter();
4343
@Output() onMenuClose = new EventEmitter();
4444

45-
constructor(private _overlayBuilder: Overlay, private _element: ElementRef,
45+
constructor(private _overlay: Overlay, private _element: ElementRef,
4646
private _viewContainerRef: ViewContainerRef) {}
4747

4848
ngAfterViewInit() {
@@ -59,46 +59,63 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
5959
}
6060

6161
openMenu(): Promise<void> {
62-
return this._overlay.attach(this._portal)
63-
.then(() => this._setMenuState(true));
62+
return this._overlayRef.attach(this._portal)
63+
.then(() => this._setIsMenuOpen(true));
6464
}
6565

6666
closeMenu(): Promise<void> {
67-
return this._overlay.detach()
68-
.then(() => this._setMenuState(false));
67+
return this._overlayRef.detach()
68+
.then(() => this._setIsMenuOpen(false));
6969
}
7070

7171
destroyMenu(): void {
72-
this._overlay.dispose();
72+
this._overlayRef.dispose();
7373
}
7474

7575
// set state rather than toggle to support triggers sharing a menu
76-
private _setMenuState(bool: boolean): void {
77-
this.menuOpen = bool;
78-
this.menu._setClickCatcher(bool);
76+
private _setIsMenuOpen(isOpen: boolean): void {
77+
this.menuOpen = isOpen;
78+
this.menu._setClickCatcher(isOpen);
7979
this.menuOpen ? this.onMenuOpen.emit(null) : this.onMenuClose.emit(null);
8080
}
8181

82+
/**
83+
* This method checks that a valid instance of MdMenu has been passed into
84+
* md-menu-trigger-for. If not, an exception is thrown.
85+
*/
8286
private _checkMenu() {
8387
if (!this.menu || !(this.menu instanceof MdMenu)) {
8488
throw new MdMenuMissingError();
8589
}
8690
}
8791

92+
/**
93+
* This method creates the overlay from the provided menu's template and saves its
94+
* OverlayRef so that it can be attached to the DOM when openMenu is called.
95+
*/
8896
private _createOverlay(): void {
8997
this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
90-
this._overlayBuilder.create(this._getOverlayConfig())
91-
.then(overlay => this._overlay = overlay);
98+
this._overlay.create(this._getOverlayConfig())
99+
.then(overlay => this._overlayRef = overlay);
92100
}
93101

102+
/**
103+
* This method builds the configuration object needed to create the overlay, the OverlayState.
104+
* @returns OverlayState
105+
*/
94106
private _getOverlayConfig(): OverlayState {
95107
const overlayState = new OverlayState();
96108
overlayState.positionStrategy = this._getPosition();
97109
return overlayState;
98110
}
99111

112+
/**
113+
* This method builds the position strategy for the overlay, so the menu is properly connected
114+
* to the trigger.
115+
* @returns ConnectedPositionStrategy
116+
*/
100117
private _getPosition(): ConnectedPositionStrategy {
101-
return this._overlayBuilder.position().connectedTo(
118+
return this._overlay.position().connectedTo(
102119
this._element,
103120
{originX: 'start', originY: 'top'},
104121
{overlayX: 'start', overlayY: 'top'}

src/components/menu/menu.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ $md-menu-side-padding: 16px !default;
2525
overflow: scroll;
2626
-webkit-overflow-scrolling: touch; // for momentum scroll on mobile
2727

28-
background: md-color($md-background, 'menu');
28+
background: md-color($md-background, 'card');
2929
}
3030

3131
[md-menu-item] {

src/components/menu/menu.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class MdMenu {
2929
/**
3030
* This function toggles the display of the menu's click catcher element.
3131
* This element covers the viewport when the menu is open to detect clicks outside the menu.
32-
* @internal
32+
* TODO: internal
3333
*/
3434
_setClickCatcher(bool: boolean): void {
3535
this._showClickCatcher = bool;

src/core/style/_palette.scss

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,6 @@ $md-light-theme-background: (
335335
hover: rgba(black, 0.04), // TODO(kara): check style with Material Design UX
336336
card: white,
337337
dialog: white,
338-
menu: white,
339338
disabled-button: rgba(black, 0.12)
340339
);
341340

@@ -347,7 +346,6 @@ $md-dark-theme-background: (
347346
hover: rgba(white, 0.04), // TODO(kara): check style with Material Design UX
348347
card: map_get($md-grey, 800),
349348
dialog: map_get($md-grey, 800),
350-
menu: map_get($md-grey, 800),
351349
disabled-button: rgba(white, 0.12)
352350
);
353351

src/demo-app/menu/menu-demo.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<div class="demo-menu">
2-
<div>
2+
<div class="menu-section">
33
<p>You clicked on: {{ selected }}</p>
44

55
<md-toolbar>
@@ -14,7 +14,7 @@
1414
</button>
1515
</md-menu>
1616
</div>
17-
<div>
17+
<div class="menu-section">
1818
<p> Clicking these will navigate:</p>
1919
<md-toolbar>
2020
<button md-icon-button [md-menu-trigger-for]="anchorMenu">

src/demo-app/menu/menu-demo.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
.demo-menu {
22
display: flex;
33

4-
div {
4+
.menu-section {
55
width: 300px;
66
margin: 20px;
77
}

0 commit comments

Comments
 (0)