-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(button): Add support for increased touch target to button. #4948
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4948 +/- ##
========================================
Coverage 98.69% 98.69%
========================================
Files 122 122
Lines 5983 5983
Branches 790 790
========================================
Hits 5905 5905
Misses 77 77
Partials 1 1 Continue to review full report at Codecov.
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
🤖 Beep boop! Screenshot test report 🚦16 screenshots changed from |
🤖 Beep boop! Screenshot test report 🚦16 screenshots changed from |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
All 708 screenshot tests passed for commit 54ec7cb 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, minor comment.
|
||
### Accessibility | ||
|
||
Material Design spec advises that touch targets should be at least 48 x 48 px. |
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.
nit: it is just 48px
height for this component.
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.
Buttons will always have at least 48px width since we add 24px padding-left/right, so this is still valid. Going to keep this to be consistent with Material spec, to avoid any confusion.
No description provided.