Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

feat(menu): expose close method on element scope; deprecate $mdOpenMenu #9193

Merged
merged 1 commit into from Oct 14, 2016
Merged

feat(menu): expose close method on element scope; deprecate $mdOpenMenu #9193

merged 1 commit into from Oct 14, 2016

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jul 31, 2016 •
edited

  • Deprecates the $mdOpenMenu method in favor of $mdMenu.open in order to simplify the API.
  • Exposes the $mdMenu.close method on the menu's scope, allowing for custom closing behavior.

Fixes #8446.

@crisbeto crisbeto added the needs: review This PR is waiting on review from the team label Jul 31, 2016
devversion
devversion reviewed Aug 1, 2016
View reviewed changes
src/components/menu/js/menuDirective.js
* You can then close the menu programatically by injecting `$mdMenu` and calling
* `$mdMenu.hide()`.
* You can then close the menu either by using `$mdCloseMenu()` in the template,
* or programatically by injecting `$mdMenu` and calling `$mdMenu.hide()`.
Copy link
Member

Choose a reason for hiding this comment

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

Typo in programmatically

EladBezalel
EladBezalel suggested changes Sep 14, 2016
View reviewed changes
Copy link
Member

@EladBezalel EladBezalel left a comment

Choose a reason for hiding this comment

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

Overall LGTM, can you add a demo for it?

@crisbeto
Copy link
Member Author

I had updated the usage example @EladBezalel, or do you want a "proper" demo?

@crisbeto crisbeto force-pushed the 8446/menu-close-method branch 2 times, most recently from 749cf5f to 065e8b9 Compare September 15, 2016 20:33
@crisbeto
Copy link
Member Author

@EladBezalel it's updated to have a demo showing how to use the custom triggers.

@crisbeto crisbeto added needs: work and removed needs: review This PR is waiting on review from the team labels Sep 15, 2016
@crisbeto
Copy link
Member Author

crisbeto commented Sep 15, 2016 •
edited

Marking as needs: work in order to simplify the API as agreed with @ThomasBurleson. The $mdOpenMenu and $mdCloseMenu scope methods shouldn't be exposed separately, but rather as a part of the scope.$mdMenu object. The old methods should still be supported, but show a deperacation warning.

@ThomasBurleson ThomasBurleson added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Sep 15, 2016
@EladBezalel
Copy link
Member

I think it deserves a proper demo

@crisbeto crisbeto changed the title feat(menu): expose close method on element scope feat(menu): expose close method on element scope; deprecate $mdOpenMenu Sep 17, 2016
@crisbeto crisbeto added needs: review This PR is waiting on review from the team and removed needs: work labels Sep 17, 2016
@crisbeto
Copy link
Member Author

@ThomasBurleson I've updated this to include:

  • Deprecation of the $mdOpenMenu method via a warning.
  • Include the demo requested by @EladBezalel.

@devversion
Copy link
Member

LGTM.

@crisbeto
feat(menu): expose close method on element scope; deprecate $mdOpenMenu
c4161a7 
* Deprecates the `$mdOpenMenu` method in favor of `$mdMenu.open` in order to simplify the API.
* Exposes the `$mdMenu.close` method on the menu's scope, allowing for custom closing behavior.

Fixes angular#8446.
@crisbeto crisbeto mentioned this pull request Oct 11, 2016
Closed
@ThomasBurleson ThomasBurleson added needs: presubmit and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: review This PR is waiting on review from the team labels Oct 11, 2016
@ThomasBurleson ThomasBurleson added this to the 1.1.2 milestone Oct 11, 2016
@kara kara added pr: merge ready This PR is ready for a caretaker to review and removed needs: presubmit labels Oct 14, 2016
@kara kara merged commit 1e4ba35 into angular:master Oct 14, 2016
@cmacdonnacha
Copy link

cmacdonnacha commented Feb 1, 2017 •
edited

Nice work on this. Will $mdOpenMenu break if I update from 1.1.1 to 1.1.3?

ng-mouseover="$mdOpenMenu($event)"

@crisbeto
Copy link
Member Author

crisbeto commented Feb 1, 2017

It won't break, but it will log a deprecation warning.

@cmacdonnacha
Copy link

Great thanks. I see that a breaking change has been introduced to 1.1.2. Should the minor version be bumped instead of the patch version?

@crisbeto
Copy link
Member Author

crisbeto commented Feb 1, 2017

I'm not sure, I haven't been involved with that particular change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@devversion devversion devversion approved these changes

@EladBezalel EladBezalel EladBezalel approved these changes

Assignees

@ThomasBurleson ThomasBurleson

@EladBezalel EladBezalel

Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Milestone
1.1.2
Development

Successfully merging this pull request may close these issues.

Enhancement: Support $mdCloseMenu
7 participants
@crisbeto @EladBezalel @devversion @cmacdonnacha @topherfangio @ThomasBurleson @kara