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): unable to bind to xPosition and yPosition #4213

Merged
merged 1 commit into from
May 2, 2017
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
71 changes: 31 additions & 40 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import {
AfterContentInit,
Attribute,
Component,
ContentChildren,
EventEmitter,
Expand Down Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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. */
Expand All @@ -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';
}

}
12 changes: 6 additions & 6 deletions src/lib/menu/menu-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
`);
}
}
4 changes: 2 additions & 2 deletions src/lib/menu/menu-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,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;
Expand Down
48 changes: 39 additions & 9 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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');
});

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