Skip to content

Conversation

@kara
Copy link
Contributor

@kara kara commented Jul 16, 2016

This PR adds basic menu toggling functionality (see design doc here).

For the below, clicking the button will automatically toggle the menu.

<button [md-menu-trigger-for]="menu">More</button>
<md-menu #menu="mdMenu">
   <button md-menu-item>Item one</button>
</md-menu>

Still TODO (in future PRs):

  • Animation for menu opening
  • Position setting
  • Keyboard events
  • Prevent-close attribute

@googlebot googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement labels Jul 16, 2016
@kara kara force-pushed the trigger branch 3 times, most recently from 1aa8e78 to 375a7ae Compare July 16, 2016 04:12

@HostBinding('attr.aria-disabled')
get isAriaDisabled(): string {
return this.disabled ? 'true' : 'false';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized we could do this as String(this.disabled), which is slightly shorter.

@aminebizid
Copy link

Menu shoould be closed when clicking outside or resizing the window (body:click & window:resize)
A z-index strategy should be managed in overlay class (adding a menu on a dialog, nested menus, etc.)


ngAfterViewInit() {
this._checkMenu();
this._createOverlay();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that it might be a problem in Angular Universal if we try to create the overlay on init rather than once a user interaction occurs, since under the hood the overlay creates an element with browser APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point. I'll check it out.

@jelbourn
Copy link
Member

LGTM

@kara kara added the action: merge The PR is ready for merge by the caretaker label Jul 21, 2016
@robertmesserle robertmesserle merged commit 9a32489 into angular:master Jul 21, 2016
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants