Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

mdPreventMenuClose: doesn't work with radio or checkbox type mdMenuItem #8110

Closed
diminutivesloop opened this issue Apr 19, 2016 · 5 comments
Closed

mdPreventMenuClose: doesn't work with radio or checkbox type mdMenuItem #8110

diminutivesloop opened this issue Apr 19, 2016 · 5 comments
Assignees
@topherfangio
Labels
needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community type: bug
Milestone
Deprecated

Comments

@diminutivesloop
Copy link

Setting the md-prevent-menu-close attribute on a md-button element fails to prevent the enclosing menu from closing when the button is clicked if the enclosing md-menu-item element has its type attribute set to either "radio" or "checkbox".

This issue is reproducible for Angular Material 1.0.7 across all major browsers.

http://codepen.io/anon/pen/qZKNmK
@devversion devversion self-assigned this Apr 19, 2016
devversion added a commit to devversion/material that referenced this issue Apr 19, 2016
@devversion
fix(menu): type checkbox should not affect a normal menu item.
0eeaf32 
* Currently we always wrap the menu-item content inside of a button, if the menu-item has an attribute with `type[checkbox|radio]`.
  This logic was originally designed for the `md-menu-bar` component, and not for the normal `md-menu`.

This caused confusion for developers and should be removed.

* Also removed a part of code, which was a duplicate and even not reachable, because the same code returned / exited above.

Fixes angular#8110
devversion added a commit to devversion/material that referenced this issue Apr 19, 2016
@devversion
fix(menu): type checkbox should not affect a normal menu item.
8dcc56a 
* Currently we always wrap the menu-item content inside of a button, if the menu-item has an attribute with `type[checkbox|radio]`.
  This logic was originally designed for the `md-menu-bar` component, and not for the normal `md-menu`.

This caused confusion for developers and should be removed.

* Also removed a part of code, which was a duplicate and even not reachable, because the same code returned / exited above.

Fixes angular#8110
@devversion devversion mentioned this issue Apr 19, 2016
Closed
devversion added a commit to devversion/material that referenced this issue Apr 19, 2016
@devversion
fix(menu): type checkbox should not affect a normal menu item.
3780e98 
* Currently we always wrap the menu-item content inside of a button, if the menu-item has an attribute with `type[checkbox|radio]`.
  This logic was originally designed for the `md-menu-bar` component, and not for the normal `md-menu`.

This caused confusion for developers and should be removed.

* Also removed a part of code, which was a duplicate and even not reachable, because the same code returned / exited above.

Fixes angular#8110
@devversion devversion added the pr: merge ready This PR is ready for a caretaker to review label Apr 19, 2016
@ThomasBurleson
Copy link
Contributor

Please verify this issue still exists in HEAD.

@ThomasBurleson ThomasBurleson removed the pr: merge ready This PR is ready for a caretaker to review label Apr 20, 2016
@ThomasBurleson
Copy link
Contributor

@topherfangio - it seems like this API is becoming complicated?

@ThomasBurleson ThomasBurleson added needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community type: bug labels Apr 20, 2016
@ThomasBurleson ThomasBurleson added this to the 1.1.1 milestone Apr 20, 2016
@topherfangio
Copy link
Contributor

@ThomasBurleson Perhaps, but I think this is more of an issue with attempting to reuse the menu code within the menu-bar. The API should never have been available in the plain menu IMO.

@devversion
Copy link
Member

@topherfangio Agree, when I investigated at the issue, I was wondering, why the menu-item directive was placed inside of the menuBar component.

It just shows in my opinion, that the menu-item directive is special-designed for the md-menu-bar.

@ThomasBurleson ThomasBurleson modified the milestones: 1.1.1, Backlog Apr 21, 2016
@ThomasBurleson ThomasBurleson modified the milestones: - Backlog, Deprecated May 26, 2016
ThomasBurleson pushed a commit that referenced this issue Jun 2, 2016
@devversion @ThomasBurleson
fix(menu): type checkbox should not affect a normal menu item.
560474f 
* Currently we always wrap the menu-item content inside of a button, if the menu-item has an attribute with `type[checkbox|radio]`.
  This logic was originally designed for the `md-menu-bar` component, and not for the normal `md-menu`.

This caused confusion for developers and should be removed.

* Also removed a part of code, which was a duplicate and even not reachable, because the same code returned / exited above.

Fixes #8110

Closes #8117
@ThomasBurleson
Copy link
Contributor

This issue is closed as part of our ‘Surge Focus on Material 2' efforts.
For details, see our forum posting @ http://bit.ly/1UhZyWs.

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

@topherfangio topherfangio

Labels
needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community type: bug
Projects
None yet
Milestone
Deprecated
Development

No branches or pull requests
4 participants
@topherfangio @ThomasBurleson @diminutivesloop @devversion