fix(material-experimental): menu item should not inherit parent classname #17953
Conversation
andrewseguin
added
pr: merge safe
target: patch
googlebot
added
the
cla: yes
| @@ -19,6 +19,9 @@ import {MatMenuItem as BaseMatMenuItem} from '@angular/material/menu'; | |||
| inputs: ['disabled', 'disableRipple'], | |||
| host: { | |||
| '[attr.role]': 'role', | |||
| // The MatMenuItem parent class adds `mat-menu-item` to the CSS classlist, but this should | |||
| // not be added for this MDC equivalent menu item. | |||
| '[class.mat-menu-item]': 'false', | |||
There was a problem hiding this comment.
AFAIK Ivy and ViewEngine have different behavior when it comes to resolving conflicting class bindings so I'm not sure how this will behave. Maybe we should just set the class manually in the constructor?
There was a problem hiding this comment.
I think there's a few approaches to doing this but none seem to be the ideal one. I don't particularly have a strong preference over any certain solution, and Jeremy and I just happened to land on this as the simplest. Based on what I see when running the devapp, this works fine
There was a problem hiding this comment.
The dev app runs in Ivy, it's ViewEngine that's inconsistent with how it handles conflicting bindings.
There was a problem hiding this comment.
You probably know more than I do with these conflicts, but I don't believe host bindings in the component metadata pass down at all to subclassed components. Do you know how to run the devapp in VE so I can test it out and confirm
There was a problem hiding this comment.
Hmm, good point that inheriting host didn't work on VE. In that case it should be fine. I don't know how we can run the dev app in VE.
crisbeto
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.