-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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.
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
fix(list): allow for list and list items to be disabled #17584
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How's the contrast ratio on disabled items?
@@ -300,6 +300,10 @@ mat-action-list { | |||
outline: none; | |||
} | |||
|
|||
.mat-list-item-disabled { | |||
pointer-events: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would also disable tooltips, I think we'd want to avoid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would we prevent clicking on it though? We've done something similar to support disabled tabs in the tab nav bar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the tabs thing is why I thought about this (someone mentioned it in a bug). I don't know of another way to do this other than preventDefault
/ stopPropagation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't rely on preventDefault
/stopPropagation
, because we don't have a way of ensuring that our event listeners are invoked before the user's.
5008f3a
to
1880478
Compare
Sorry for the delayed response @jelbourn, I must've missed the notification. The feedback has been addressed, can you take another look. As for the contrast ratio on disabled items, I haven't checked it but it's the same as what we have in master for selection lists. |
f776d58
to
f1d52ef
Compare
when will this get merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I guess native controls don't show their title
tooltip when disabled, so it makes sense for our stuff to do the same.
f1d52ef
to
b54fccb
Compare
Adds the ability to disable all variants of the list. Currently it's only the selection list that can be disabled. Fixes angular#17574.
b54fccb
to
a190db0
Compare
Adds the ability to disable all variants of the list. Currently it's only the selection list that can be disabled. Fixes angular#17574.
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. |
Adds the ability to disable all variants of the list. Currently it's only the selection list that can be disabled.
Fixes #17574.