Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

feat(button): Add support for increased touch target to button. #4948

Merged
merged 11 commits into from
Jul 25, 2019

Conversation

joyzhong
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Jul 24, 2019

Codecov Report

Merging #4948 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2e0fea...54ec7cb. Read the comment docs.

@googlebot
Copy link

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 @googlebot I consent. in this pull request.

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 cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@joyzhong joyzhong changed the base branch from master to develop July 24, 2019 20:41
@joyzhong joyzhong requested a review from abhiomkar July 24, 2019 20:42
@joyzhong joyzhong added cla: yes and removed cla: no labels Jul 24, 2019
@googlebot
Copy link

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.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@mdc-web-bot
Copy link
Collaborator

All 708 screenshot tests passed for commit 54ec7cb vs. develop! 💯🎉

Copy link
Collaborator

@abhiomkar abhiomkar left a 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.
Copy link
Collaborator

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.

Copy link
Contributor Author

@joyzhong joyzhong Jul 25, 2019

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.

@joyzhong joyzhong merged commit 1d7a2e6 into develop Jul 25, 2019
@joyzhong joyzhong deleted the feat/button_touchtarget branch July 25, 2019 02:16
@joyzhong joyzhong restored the feat/button_touchtarget branch July 26, 2019 18:05
@jamesmfriedman jamesmfriedman mentioned this pull request Nov 4, 2019
93 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants