-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(list): Add notifyAction adapter for action on list item. #4144
Conversation
All 691 screenshot tests passed for commit b39a6b1 vs. |
Codecov Report
@@ Coverage Diff @@
## master #4144 +/- ##
=======================================
Coverage 98.58% 98.58%
=======================================
Files 127 127
Lines 5705 5705
Branches 762 762
=======================================
Hits 5624 5624
Misses 81 81
Continue to review full report at Codecov.
|
All 691 screenshot tests passed for commit 1d4f896 vs. |
All 691 screenshot tests passed for commit a7e8edd vs. |
All 691 screenshot tests passed for commit 5c93287 vs. |
All 758 screenshot tests passed for commit 20d069e vs. |
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, presuming tests pass now
PS: I think this should probably be considered a |
All 758 screenshot tests passed for commit f127152 vs. |
notifyAction
adapter method is introduced to notify on user action including keyboard and mouse. This could've beennotifySelected
but the a list can also be a checkbox variant where it can have multiple selections and an action could also cause unselect.BREAKING CHANGE: Removed adapter method
followHref
and used native anchor element behaviour to follow href on Enter & click. Components that use MDC List should use its new custom event.Changes required for #3481