fix(menu): unable to bind to xPosition and yPosition (#4213)

Fixes not being able to use data bindings on the `xPosition` and `yPosition` properties.

**Note:** These changes could be considered breaking, if somebody was implementing the `MdMenuPanel` interface.

Fixes #4169.

Author: crisbeto

src/lib/menu/menu-directive.ts

@@ -2,7 +2,6 @@
 
 import {
   AfterContentInit,
-  Attribute,
   Component,
   ContentChildren,
   EventEmitter,
@@ -37,6 +36,8 @@ import {transformMenu, fadeInItems} from './menu-animations';
 })
 export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
   private _keyManager: FocusKeyManager;
+  private _xPosition: MenuPositionX = 'after';
+  private _yPosition: MenuPositionY = 'below';
 
   /** Subscription to tab events on the menu panel */
   private _tabSubscription: Subscription;
@@ -45,34 +46,38 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
   _classList: any = {};
 
   /** Position of the menu in the X axis. */
-  positionX: MenuPositionX = 'after';
+  @Input()
+  get xPosition() { return this._xPosition; }
+  set xPosition(value: MenuPositionX) {
+    if (value !== 'before' && value !== 'after') {
+      throw new MdMenuInvalidPositionX();
+    }
+    this._xPosition = value;
+    this.setPositionClasses();
+  }
 
   /** Position of the menu in the Y axis. */
-  positionY: MenuPositionY = 'below';
+  @Input()
+  get yPosition() { return this._yPosition; }
+  set yPosition(value: MenuPositionY) {
+    if (value !== 'above' && value !== 'below') {
+      throw new MdMenuInvalidPositionY();
+    }
+    this._yPosition = value;
+    this.setPositionClasses();
+  }
 
   @ViewChild(TemplateRef) templateRef: TemplateRef<any>;
-  @ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
-  @Input() overlapTrigger = true;
 
-  constructor(@Attribute('xPosition') posX: MenuPositionX,
-              @Attribute('yPosition') posY: MenuPositionY,
-              @Attribute('x-position') deprecatedPosX: MenuPositionX,
-              @Attribute('y-position') deprecatedPosY: MenuPositionY) {
-
-    // TODO(kara): Remove kebab-case attributes after next release
-    if (deprecatedPosX) { this._setPositionX(deprecatedPosX); }
-    if (deprecatedPosY) { this._setPositionY(deprecatedPosY); }
+  /** List of the items inside of a menu. */
+  @ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
 
-    if (posX) { this._setPositionX(posX); }
-    if (posY) { this._setPositionY(posY); }
-    this.setPositionClasses(this.positionX, this.positionY);
-  }
+  /** Whether the menu should overlap its trigger. */
+  @Input() overlapTrigger = true;
 
   ngAfterContentInit() {
     this._keyManager = new FocusKeyManager(this.items).withWrap();
-    this._tabSubscription = this._keyManager.tabOut.subscribe(() => {
-      this._emitCloseEvent();
-    });
+    this._tabSubscription = this._keyManager.tabOut.subscribe(() => this._emitCloseEvent());
   }
 
   ngOnDestroy() {
@@ -93,7 +98,7 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
       obj[className] = true;
       return obj;
     }, {});
-    this.setPositionClasses(this.positionX, this.positionY);
+    this.setPositionClasses();
   }
 
   /** Event emitted when the menu is closed. */
@@ -115,29 +120,15 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
     this.close.emit();
   }
 
-  private _setPositionX(pos: MenuPositionX): void {
-    if (pos !== 'before' && pos !== 'after') {
-      throw new MdMenuInvalidPositionX();
-    }
-    this.positionX = pos;
-  }
-
-  private _setPositionY(pos: MenuPositionY): void {
-    if (pos !== 'above' && pos !== 'below') {
-      throw new MdMenuInvalidPositionY();
-    }
-    this.positionY = pos;
-  }
-
   /**
    * It's necessary to set position-based classes to ensure the menu panel animation
    * folds out from the correct direction.
    */
-  setPositionClasses(posX: MenuPositionX, posY: MenuPositionY): void {
-    this._classList['mat-menu-before'] = posX == 'before';
-    this._classList['mat-menu-after'] = posX == 'after';
-    this._classList['mat-menu-above'] = posY == 'above';
-    this._classList['mat-menu-below'] = posY == 'below';
+  setPositionClasses(posX = this.xPosition, posY = this.yPosition): void {
+    this._classList['mat-menu-before'] = posX === 'before';
+    this._classList['mat-menu-after'] = posX === 'after';
+    this._classList['mat-menu-above'] = posY === 'above';
+    this._classList['mat-menu-below'] = posY === 'below';
   }
 
 }

src/lib/menu/menu-errors.ts

@@ -16,27 +16,27 @@ export class MdMenuMissingError extends MdError {
 }
 
 /**
- * Exception thrown when menu's x-position value isn't valid.
+ * Exception thrown when menu's xPosition value isn't valid.
  * In other words, it doesn't match 'before' or 'after'.
  * @docs-private
  */
 export class MdMenuInvalidPositionX extends MdError {
   constructor() {
-    super(`x-position value must be either 'before' or after'.
-      Example: <md-menu x-position="before" #menu="mdMenu"></md-menu>
+    super(`xPosition value must be either 'before' or after'.
+      Example: <md-menu xPosition="before" #menu="mdMenu"></md-menu>
     `);
   }
 }
 
 /**
- * Exception thrown when menu's y-position value isn't valid.
+ * Exception thrown when menu's yPosition value isn't valid.
  * In other words, it doesn't match 'above' or 'below'.
  * @docs-private
  */
 export class MdMenuInvalidPositionY extends MdError {
   constructor() {
-    super(`y-position value must be either 'above' or below'.
-      Example: <md-menu y-position="above" #menu="mdMenu"></md-menu>
+    super(`yPosition value must be either 'above' or below'.
+      Example: <md-menu yPosition="above" #menu="mdMenu"></md-menu>
     `);
   }
 }

src/lib/menu/menu-panel.ts

@@ -2,8 +2,8 @@ import {EventEmitter, TemplateRef} from '@angular/core';
 import {MenuPositionX, MenuPositionY} from './menu-positions';
 
 export interface MdMenuPanel {
-  positionX: MenuPositionX;
-  positionY: MenuPositionY;
+  xPosition: MenuPositionX;
+  yPosition: MenuPositionY;
   overlapTrigger: boolean;
   templateRef: TemplateRef<any>;
   close: EventEmitter<void>;

src/lib/menu/menu-trigger.ts

@@ -244,10 +244,10 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
    */
   private _getPosition(): ConnectedPositionStrategy  {
     const [posX, fallbackX]: HorizontalConnectionPos[] =
-      this.menu.positionX === 'before' ? ['end', 'start'] : ['start', 'end'];
+      this.menu.xPosition === 'before' ? ['end', 'start'] : ['start', 'end'];
 
     const [overlayY, fallbackOverlayY]: VerticalConnectionPos[] =
-      this.menu.positionY === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];
+      this.menu.yPosition === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];
 
     let originY = overlayY;
     let fallbackOriginY = fallbackOverlayY;

src/lib/menu/menu.spec.ts

@@ -108,10 +108,13 @@ describe('MdMenu', () => {
   });
 
   describe('positions', () => {
+    let fixture: ComponentFixture<PositionedMenu>;
+    let panel: HTMLElement;
 
     beforeEach(() => {
-      const fixture = TestBed.createComponent(PositionedMenu);
+      fixture = TestBed.createComponent(PositionedMenu);
       fixture.detectChanges();
+
       const trigger = fixture.componentInstance.triggerEl.nativeElement;
 
       // Push trigger to the bottom edge of viewport,so it has space to open "above"
@@ -123,18 +126,43 @@ describe('MdMenu', () => {
 
       fixture.componentInstance.trigger.openMenu();
       fixture.detectChanges();
+      panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
     });
 
-    it('should append mat-menu-before if x position is changed', () => {
-      const panel = overlayContainerElement.querySelector('.mat-menu-panel');
+    it('should append mat-menu-before if the x position is changed', () => {
       expect(panel.classList).toContain('mat-menu-before');
       expect(panel.classList).not.toContain('mat-menu-after');
+
+      fixture.componentInstance.xPosition = 'after';
+      fixture.detectChanges();
+
+      expect(panel.classList).toContain('mat-menu-after');
+      expect(panel.classList).not.toContain('mat-menu-before');
     });
 
-    it('should append mat-menu-above if y position is changed', () => {
-      const panel = overlayContainerElement.querySelector('.mat-menu-panel');
+    it('should append mat-menu-above if the y position is changed', () => {
       expect(panel.classList).toContain('mat-menu-above');
       expect(panel.classList).not.toContain('mat-menu-below');
+
+      fixture.componentInstance.yPosition = 'below';
+      fixture.detectChanges();
+
+      expect(panel.classList).toContain('mat-menu-below');
+      expect(panel.classList).not.toContain('mat-menu-above');
+    });
+
+    it('should default to the "below" and "after" positions', () => {
+      fixture.destroy();
+
+      let newFixture = TestBed.createComponent(SimpleMenu);
+
+      newFixture.detectChanges();
+      newFixture.componentInstance.trigger.openMenu();
+      newFixture.detectChanges();
+      panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
+
+      expect(panel.classList).toContain('mat-menu-below');
+      expect(panel.classList).toContain('mat-menu-after');
     });
 
   });
@@ -193,7 +221,7 @@ describe('MdMenu', () => {
           .toBe(Math.round(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
+      // The xPosition of the overlay should be unaffected, as it can already fit horizontally
       expect(Math.round(overlayRect.left))
           .toBe(Math.round(triggerRect.left),
               `Expected menu x position to be unchanged if it can fit in the viewport.`);
@@ -437,14 +465,16 @@ class SimpleMenu {
 @Component({
   template: `
     <button [mdMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
-    <md-menu x-position="before" y-position="above" #menu="mdMenu">
+    <md-menu [xPosition]="xPosition" [yPosition]="yPosition" #menu="mdMenu">
       <button md-menu-item> Positioned Content </button>
     </md-menu>
   `
 })
 class PositionedMenu {
   @ViewChild(MdMenuTrigger) trigger: MdMenuTrigger;
   @ViewChild('triggerEl') triggerEl: ElementRef;
+  xPosition: MenuPositionX = 'before';
+  yPosition: MenuPositionY = 'above';
 }
 
 interface TestableMenu {
@@ -476,8 +506,8 @@ class OverlapMenu implements TestableMenu {
   exportAs: 'mdCustomMenu'
 })
 class CustomMenuPanel implements MdMenuPanel {
-  positionX: MenuPositionX = 'after';
-  positionY: MenuPositionY = 'below';
+  xPosition: MenuPositionX = 'after';
+  yPosition: MenuPositionY = 'below';
   overlapTrigger: true;
 
   @ViewChild(TemplateRef) templateRef: TemplateRef<any>;