Skip to content
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(component): create mobileFriendly prop #413

Closed
wants to merge 1 commit into from

Conversation

kchu93
Copy link
Contributor

@kchu93 kchu93 commented Jun 23, 2020

What

Create mobileFriendly prop.

Screenshot

Default button (mobileFriendly not set)
Screen Shot 2020-06-23 at 9 55 55 AM

With mobileFriendly Prop true
Screen Shot 2020-06-23 at 9 55 05 AM

Fixes #388

@kchu93 kchu93 requested a review from a team as a code owner June 23, 2020 16:58
Copy link
Contributor

@chanceaclark chanceaclark left a comment

Choose a reason for hiding this comment

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

A couple notes:

  1. I'm not sure mobileFriendly makes for a good API name. Could you come up and propose a list of names instead?
  2. This will be a breaking change. Since that's the case you will need to add commit messaging for it so it generates the appropriate changelog.
  3. You also need to add the prop to the corresponding PropTable(s).
  4. We should also optimize the piece of code (Line 59) that changes for the media query based on the prop.
  5. Even though the snapshots will/did get updated, you should go through and manually double-check check to see components using StyleableButton or Button didn't break on each device type.

@kchu93
Copy link
Contributor Author

kchu93 commented Jun 23, 2020

Going to close this PR out for now, believe we have just tabled this for now, can always reopen once we have a solid plan.

@kchu93 kchu93 closed this Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Buttons - Create a "Mobile Friendly" prop
2 participants