Skip to content

Commit 582c2a3

Browse files
crisbetocrisbeto
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 582c2a3

File tree

5 files changed

+80
-58
lines changed

5 files changed

+80
-58
lines changed

‎src/lib/menu/menu-directive.ts‎

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import {transformMenu, fadeInItems} from './menu-animations';
3737
})
3838
export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
3939
private _keyManager: FocusKeyManager;
40+
private _xPosition: MenuPositionX = 'after';
41+
private _yPosition: MenuPositionY = 'below';
4042

4143
/** Subscription to tab events on the menu panel */
4244
private _tabSubscription: Subscription;
@@ -45,34 +47,38 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
4547
_classList: any = {};
4648

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

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

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

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); }
73+
/** List of the items inside of a menu. */
74+
@ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
6575

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

7179
ngAfterContentInit() {
7280
this._keyManager = new FocusKeyManager(this.items).withWrap();
73-
this._tabSubscription = this._keyManager.tabOut.subscribe(() => {
74-
this._emitCloseEvent();
75-
});
81+
this._tabSubscription = this._keyManager.tabOut.subscribe(() => this._emitCloseEvent());
7682
}
7783

7884
ngOnDestroy() {
@@ -93,7 +99,7 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
9399
obj[className] = true;
94100
return obj;
95101
}, {});
96-
this.setPositionClasses(this.positionX, this.positionY);
102+
this.setPositionClasses();
97103
}
98104

99105
/** Event emitted when the menu is closed. */
@@ -115,29 +121,15 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
115121
this.close.emit();
116122
}
117123

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-
132124
/**
133125
* It's necessary to set position-based classes to ensure the menu panel animation
134126
* folds out from the correct direction.
135127
*/
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';
128+
setPositionClasses(posX = this.xPosition, posY = this.yPosition): void {
129+
this._classList['mat-menu-before'] = posX === 'before';
130+
this._classList['mat-menu-after'] = posX === 'after';
131+
this._classList['mat-menu-above'] = posY === 'above';
132+
this._classList['mat-menu-below'] = posY === 'below';
141133
}
142134

143135
}

‎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)