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

fix(menu): type checkbox should not affect a normal menu item. #8117

Closed
wants to merge 1 commit into from

Conversation

devversion
Copy link
Member

  • 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

@devversion devversion added the needs: review This PR is waiting on review from the team label Apr 19, 2016
* 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 force-pushed the fix/menu-type-checkbox branch from 8dcc56a to 3780e98 Compare April 19, 2016 18:55
@topherfangio topherfangio added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Apr 19, 2016
@topherfangio
Copy link
Contributor

LGTM 👍

@ThomasBurleson ThomasBurleson modified the milestones: 1.2.0, Backlog Apr 20, 2016
@devversion devversion deleted the fix/menu-type-checkbox branch June 2, 2016 10:13
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.

Referencing angular#8117.

Fixes angular#8697.
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.

Referencing angular#8117.

Fixes angular#8697.
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.

Referencing angular#8117.

Fixes angular#8697.
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.
* Switches to using the prefixer in the menuItem directive.

Referencing angular#8117.

Fixes angular#8697.
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.
* Switches to using the prefixer in the menuItem directive.

Referencing angular#8117.

Fixes angular#8697.
crisbeto added a commit to crisbeto/material that referenced this pull request Jun 25, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.
* Switches to using the prefixer in the menuItem directive.

Referencing angular#8117.

Fixes angular#8697.
ThomasBurleson pushed a commit that referenced this pull request Jul 10, 2016
* Fixes the menuItem directive not compiling properly when used together with ngRepeat. This is due to the fact that ngRepeat passes around a documentFragment, instead of a DOM node, which means that it doesn't have a parent and doesn't allow us to properly determine whether the menuItem is inside of a menuBar. This change switches the behavior by making the menuBar directive mark it's children instead.
* Minor cleanup in the menuItem directive.
* Avoided some repetition in the menuBar unit tests.
* Switches to using the prefixer in the menuItem directive.

Referencing #8117.

Fixes #8697.

Closes #8852

Closes #8850
@Splaktar Splaktar removed this from the - Backlog milestone Jun 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants