Skip to content

Commit e14d91d

Browse files
committed
fix(menu): unable to bind to xPosition and yPosition
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.
1 parent 66e65c4 commit e14d91d

File tree

5 files changed

+80
-59
lines changed

5 files changed

+80
-59
lines changed

src/lib/menu/menu-directive.ts

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import {
44
AfterContentInit,
5-
Attribute,
65
Component,
76
ContentChildren,
87
EventEmitter,
@@ -37,6 +36,8 @@ import {transformMenu, fadeInItems} from './menu-animations';
3736
})
3837
export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
3938
private _keyManager: FocusKeyManager;
39+
private _xPosition: MenuPositionX = 'after';
40+
private _yPosition: MenuPositionY = 'below';
4041

4142
/** Subscription to tab events on the menu panel */
4243
private _tabSubscription: Subscription;
@@ -45,34 +46,38 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
4546
_classList: any = {};
4647

4748
/** Position of the menu in the X axis. */
48-
positionX: MenuPositionX = 'after';
49+
@Input()
50+
get xPosition() { return this._xPosition; }
51+
set xPosition(value: MenuPositionX) {
52+
if (value !== 'before' && value !== 'after') {
53+
throw new MdMenuInvalidPositionX();
54+
}
55+
this._xPosition = value;
56+
this.setPositionClasses();
57+
}
4958

5059
/** Position of the menu in the Y axis. */
51-
positionY: MenuPositionY = 'below';
60+
@Input()
61+
get yPosition() { return this._yPosition; }
62+
set yPosition(value: MenuPositionY) {
63+
if (value !== 'above' && value !== 'below') {
64+
throw new MdMenuInvalidPositionY();
65+
}
66+
this._yPosition = value;
67+
this.setPositionClasses();
68+
}
5269

5370
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
54-
@ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
55-
@Input() overlapTrigger = true;
5671

57-
constructor(@Attribute('xPosition') posX: MenuPositionX,
58-
@Attribute('yPosition') posY: MenuPositionY,
59-
@Attribute('x-position') deprecatedPosX: MenuPositionX,
60-
@Attribute('y-position') deprecatedPosY: MenuPositionY) {
61-
62-
// TODO(kara): Remove kebab-case attributes after next release
63-
if (deprecatedPosX) { this._setPositionX(deprecatedPosX); }
64-
if (deprecatedPosY) { this._setPositionY(deprecatedPosY); }
72+
/** List of the items inside of a menu. */
73+
@ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
6574

66-
if (posX) { this._setPositionX(posX); }
67-
if (posY) { this._setPositionY(posY); }
68-
this.setPositionClasses(this.positionX, this.positionY);
69-
}
75+
/** Whether the menu should overlap its trigger. */
76+
@Input() overlapTrigger = true;
7077

7178
ngAfterContentInit() {
7279
this._keyManager = new FocusKeyManager(this.items).withWrap();
73-
this._tabSubscription = this._keyManager.tabOut.subscribe(() => {
74-
this._emitCloseEvent();
75-
});
80+
this._tabSubscription = this._keyManager.tabOut.subscribe(() => this._emitCloseEvent());
7681
}
7782

7883
ngOnDestroy() {
@@ -93,7 +98,7 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
9398
obj[className] = true;
9499
return obj;
95100
}, {});
96-
this.setPositionClasses(this.positionX, this.positionY);
101+
this.setPositionClasses();
97102
}
98103

99104
/** Event emitted when the menu is closed. */
@@ -115,29 +120,15 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
115120
this.close.emit();
116121
}
117122

118-
private _setPositionX(pos: MenuPositionX): void {
119-
if (pos !== 'before' && pos !== 'after') {
120-
throw new MdMenuInvalidPositionX();
121-
}
122-
this.positionX = pos;
123-
}
124-
125-
private _setPositionY(pos: MenuPositionY): void {
126-
if (pos !== 'above' && pos !== 'below') {
127-
throw new MdMenuInvalidPositionY();
128-
}
129-
this.positionY = pos;
130-
}
131-
132123
/**
133124
* It's necessary to set position-based classes to ensure the menu panel animation
134125
* folds out from the correct direction.
135126
*/
136-
setPositionClasses(posX: MenuPositionX, posY: MenuPositionY): void {
137-
this._classList['mat-menu-before'] = posX == 'before';
138-
this._classList['mat-menu-after'] = posX == 'after';
139-
this._classList['mat-menu-above'] = posY == 'above';
140-
this._classList['mat-menu-below'] = posY == 'below';
127+
setPositionClasses(posX = this.xPosition, posY = this.yPosition): void {
128+
this._classList['mat-menu-before'] = posX === 'before';
129+
this._classList['mat-menu-after'] = posX === 'after';
130+
this._classList['mat-menu-above'] = posY === 'above';
131+
this._classList['mat-menu-below'] = posY === 'below';
141132
}
142133

143134
}

src/lib/menu/menu-errors.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,27 @@ export class MdMenuMissingError extends MdError {
1616
}
1717

1818
/**
19-
* Exception thrown when menu's x-position value isn't valid.
19+
* Exception thrown when menu's xPosition value isn't valid.
2020
* In other words, it doesn't match 'before' or 'after'.
2121
* @docs-private
2222
*/
2323
export class MdMenuInvalidPositionX extends MdError {
2424
constructor() {
25-
super(`x-position value must be either 'before' or after'.
26-
Example: <md-menu x-position="before" #menu="mdMenu"></md-menu>
25+
super(`xPosition value must be either 'before' or after'.
26+
Example: <md-menu xPosition="before" #menu="mdMenu"></md-menu>
2727
`);
2828
}
2929
}
3030

3131
/**
32-
* Exception thrown when menu's y-position value isn't valid.
32+
* Exception thrown when menu's yPosition value isn't valid.
3333
* In other words, it doesn't match 'above' or 'below'.
3434
* @docs-private
3535
*/
3636
export class MdMenuInvalidPositionY extends MdError {
3737
constructor() {
38-
super(`y-position value must be either 'above' or below'.
39-
Example: <md-menu y-position="above" #menu="mdMenu"></md-menu>
38+
super(`yPosition value must be either 'above' or below'.
39+
Example: <md-menu yPosition="above" #menu="mdMenu"></md-menu>
4040
`);
4141
}
4242
}

src/lib/menu/menu-panel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import {EventEmitter, TemplateRef} from '@angular/core';
22
import {MenuPositionX, MenuPositionY} from './menu-positions';
33

44
export interface MdMenuPanel {
5-
positionX: MenuPositionX;
6-
positionY: MenuPositionY;
5+
xPosition: MenuPositionX;
6+
yPosition: MenuPositionY;
77
overlapTrigger: boolean;
88
templateRef: TemplateRef<any>;
99
close: EventEmitter<void>;

src/lib/menu/menu-trigger.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,10 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
246246
*/
247247
private _getPosition(): ConnectedPositionStrategy {
248248
const [posX, fallbackX]: HorizontalConnectionPos[] =
249-
this.menu.positionX === 'before' ? ['end', 'start'] : ['start', 'end'];
249+
this.menu.xPosition === 'before' ? ['end', 'start'] : ['start', 'end'];
250250

251251
const [overlayY, fallbackOverlayY]: VerticalConnectionPos[] =
252-
this.menu.positionY === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];
252+
this.menu.yPosition === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];
253253

254254
let originY = overlayY;
255255
let fallbackOriginY = fallbackOverlayY;

src/lib/menu/menu.spec.ts

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,13 @@ describe('MdMenu', () => {
108108
});
109109

110110
describe('positions', () => {
111+
let fixture: ComponentFixture<PositionedMenu>;
112+
let panel: HTMLElement;
111113

112114
beforeEach(() => {
113-
const fixture = TestBed.createComponent(PositionedMenu);
115+
fixture = TestBed.createComponent(PositionedMenu);
114116
fixture.detectChanges();
117+
115118
const trigger = fixture.componentInstance.triggerEl.nativeElement;
116119

117120
// Push trigger to the bottom edge of viewport,so it has space to open "above"
@@ -123,18 +126,43 @@ describe('MdMenu', () => {
123126

124127
fixture.componentInstance.trigger.openMenu();
125128
fixture.detectChanges();
129+
panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
126130
});
127131

128-
it('should append mat-menu-before if x position is changed', () => {
129-
const panel = overlayContainerElement.querySelector('.mat-menu-panel');
132+
it('should append mat-menu-before if the x position is changed', () => {
130133
expect(panel.classList).toContain('mat-menu-before');
131134
expect(panel.classList).not.toContain('mat-menu-after');
135+
136+
fixture.componentInstance.xPosition = 'after';
137+
fixture.detectChanges();
138+
139+
expect(panel.classList).toContain('mat-menu-after');
140+
expect(panel.classList).not.toContain('mat-menu-before');
132141
});
133142

134-
it('should append mat-menu-above if y position is changed', () => {
135-
const panel = overlayContainerElement.querySelector('.mat-menu-panel');
143+
it('should append mat-menu-above if the y position is changed', () => {
136144
expect(panel.classList).toContain('mat-menu-above');
137145
expect(panel.classList).not.toContain('mat-menu-below');
146+
147+
fixture.componentInstance.yPosition = 'below';
148+
fixture.detectChanges();
149+
150+
expect(panel.classList).toContain('mat-menu-below');
151+
expect(panel.classList).not.toContain('mat-menu-above');
152+
});
153+
154+
it('should default to the "below" and "after" positions', () => {
155+
fixture.destroy();
156+
157+
let newFixture = TestBed.createComponent(SimpleMenu);
158+
159+
newFixture.detectChanges();
160+
newFixture.componentInstance.trigger.openMenu();
161+
newFixture.detectChanges();
162+
panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
163+
164+
expect(panel.classList).toContain('mat-menu-below');
165+
expect(panel.classList).toContain('mat-menu-after');
138166
});
139167

140168
});
@@ -193,7 +221,7 @@ describe('MdMenu', () => {
193221
.toBe(Math.round(expectedTop),
194222
`Expected menu to open in "above" position if "below" position wouldn't fit.`);
195223

196-
// The x-position of the overlay should be unaffected, as it can already fit horizontally
224+
// The xPosition of the overlay should be unaffected, as it can already fit horizontally
197225
expect(Math.round(overlayRect.left))
198226
.toBe(Math.round(triggerRect.left),
199227
`Expected menu x position to be unchanged if it can fit in the viewport.`);
@@ -437,14 +465,16 @@ class SimpleMenu {
437465
@Component({
438466
template: `
439467
<button [mdMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
440-
<md-menu x-position="before" y-position="above" #menu="mdMenu">
468+
<md-menu [xPosition]="xPosition" [yPosition]="yPosition" #menu="mdMenu">
441469
<button md-menu-item> Positioned Content </button>
442470
</md-menu>
443471
`
444472
})
445473
class PositionedMenu {
446474
@ViewChild(MdMenuTrigger) trigger: MdMenuTrigger;
447475
@ViewChild('triggerEl') triggerEl: ElementRef;
476+
xPosition: MenuPositionX = 'before';
477+
yPosition: MenuPositionY = 'above';
448478
}
449479

450480
interface TestableMenu {
@@ -476,8 +506,8 @@ class OverlapMenu implements TestableMenu {
476506
exportAs: 'mdCustomMenu'
477507
})
478508
class CustomMenuPanel implements MdMenuPanel {
479-
positionX: MenuPositionX = 'after';
480-
positionY: MenuPositionY = 'below';
509+
xPosition: MenuPositionX = 'after';
510+
yPosition: MenuPositionY = 'below';
481511
overlapTrigger: true;
482512

483513
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;

0 commit comments

Comments
 (0)