-
Notifications
You must be signed in to change notification settings - Fork 50
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
Colorful #278
Colorful #278
Conversation
Could you review this? @suda |
Hi @aminya I'm currently away for two weeks and unable to test it. Is it ok if I to it after I'm back? |
Yes sure! Thanks |
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.
Implementation looks good, I have some comments (mainly about the documentation) to improve the Pull Request 👍
Ps. you may consider adding your contributions to the CHANGELOG
https: //github.com//pull/278#discussion_r383049433 https: //github.com//pull/278#discussion_r383049507 https: //github.com//pull/278#discussion_r383049559 https: //github.com//pull/278#discussion_r383049807 https: //github.com//pull/278#discussion_r383050133 https: //github.com//pull/278#discussion_r383050142 Co-Authored-By: Eric Cornelissen <ericornelissen@gmail.com>
All are fixed in 1f0e0bb. Thanks for the comments. |
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.
Thanks @aminya 👍 Now this looks good to me, the final decision is up to suda
Thanks for the review Eric! Merging in :) |
Adds options for button color and background-color
Derived from #21