-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Refactor Button Component #492
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
Conversation
|
@binoy14 @iRoachie thoughts on this? 🤔 Basically, here's what I had in mind while refactoring:
Here's an example of what a button looks like: This will def break existing functionality, but I would rather have a cleaner and simpler API to work with for Also here are some useful designs to implement in case anyone wants to play with the
p.s - if this get's approved, many more breaking changes and refactors to come. ;) |
| @@ -1,281 +1,182 @@ | |||
| import PropTypes from 'prop-types'; | |||
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 is the file that needs to be review for the API changes
|
+1 for the icon change, hate passing an object for this. |
|
Thanks for the feedback @nonotest! |
|
This includes breaking changes but i think it is worth it. I really like the way you use |
|
@nonotest @xavier-villelegier thanks for pitching in. I used this API myself to design buttons on the Also I have added a few todo items here. |
Major Breaking changes
Please see the
/exampleapp for the above implementation.Really simple and useful API: