Skip to content

feat(material/menu): enhance menu item component with interactive disabled state #30730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(material/menu): enhance menu item component with interactive disabled state #30730

wants to merge 1 commit into from

Conversation

mistrykaran91
Copy link
Contributor

@mistrykaran91 mistrykaran91 commented Mar 27, 2025 •
edited
Loading

In menu-item component add an option disabledInteractive to interact with menu item in disabled state similar to mat-button, mat-radio, etc

Fixes #29984

@mistrykaran91 mistrykaran91 requested a review from a team as a code owner March 27, 2025 13:06
@mistrykaran91 mistrykaran91 requested review from crisbeto and wagnermaciel and removed request for a team March 27, 2025 13:06
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: material/menu labels Mar 27, 2025
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from a61c264 to 90b1bf7 Compare March 27, 2025 13:14
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from 90b1bf7 to fe167d5 Compare April 7, 2025 04:29
crisbeto
crisbeto reviewed Apr 7, 2025
View reviewed changes
src/material/menu/menu.scss Outdated
@@ -178,6 +180,7 @@ mat-menu {
&[disabled] {
cursor: default;
opacity: 0.38;
pointer-events: none;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being added? The browser already prevents interaction with disabled elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

I had to include that like button-toggle.scss since, the tooltip would have been shown even if disabledInteractive was set to false.

Copy link
Member

Choose a reason for hiding this comment

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

The point of disableInteractive is that the item should look disabled, but things like tooltips should still show up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I got that but the tooltip is displayed even when the disabledInteractive is set to false and I think it should be shown only in case of disabledInteractive being set to true. Similar to checkbox and radio, they do not display tooltip of disabledInteractive is set to false, Sorry if was bit unclear :)

Copy link
Member

Choose a reason for hiding this comment

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

That shouldn't happen on button nodes. For anchors it's fine since the browser doesn't really have a concept of them being disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, then I remove that style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Copy link
Contributor Author

@mistrykaran91 mistrykaran91 Apr 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@crisbeto If you see the below example, here also in disabled button with mat-tooltip and no disabledInteractive tooltip is not displayed. So, shouldn't menu button also be similar to this or the other way round.
https://material.angular.io/components/button/examples#button-disabled-interactive

image

Also, I see here also we have pointer-events:none

Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting, I actually forgot about that I introduced that in #19183. I guess that functionality already resolves #29984 since the user was asking for tooltips.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, its with ::after pseudo selector now. So, we close this or have it with the background color being different on disabledInteractive hover ?

@mistrykaran91 mistrykaran91 requested a review from crisbeto April 7, 2025 06:37
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from 2fccb47 to 80b9481 Compare April 7, 2025 07:58
@crisbeto crisbeto added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Apr 7, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 7, 2025 •
edited
Loading

Deployed dev-app for 36221d3 to: https://ng-dev-previews-comp--pr-angular-components-30730-dev-ry2lo0eh.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from 80b9481 to dc9498b Compare April 7, 2025 08:49
crisbeto
crisbeto reviewed Apr 8, 2025
View reviewed changes
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from dc9498b to 93c0f60 Compare April 8, 2025 10:06
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from 93c0f60 to cf09508 Compare April 8, 2025 10:09
@mistrykaran91
feat(material/menu): enhance menu item component with interactive dis…
36221d3 
…abled state

In menu-item component add an option `disabledInteractive` to interact with menu item in disabled state similar to `mat-button`, `mat-radio`, etc

Fixes angular#29984
@mistrykaran91 mistrykaran91 force-pushed the feat/add-disabledInteractive-option-for-mat-menu-item branch from cf09508 to 36221d3 Compare April 8, 2025 10:20
@mistrykaran91 mistrykaran91 requested a review from crisbeto April 8, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wagnermaciel wagnermaciel Awaiting requested review from wagnermaciel wagnermaciel is a code owner automatically assigned from angular/components-googlers

@crisbeto crisbeto Awaiting requested review from crisbeto

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area: material/chips area: material/menu detected: feature PR contains a feature commit dev-app preview When applied, previews of the dev-app are deployed to Firebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(COMPONENT): add "disabledInteractive" input to "mat-menu-item"
2 participants
@mistrykaran91 @crisbeto