This repository has been archived by the owner on Mar 4, 2020. It is now read-only.
feat(Button): add text variant #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mnajdova
requested review from
dvdzkwsk,
kuzhelov,
levithomason and
miroslavstastny
as code owners
Codecov Report
@@ Coverage Diff @@
## master #177 +/- ##
=========================================
- Coverage 67.69% 67.09% -0.6%
=========================================
Files 103 101 -2
Lines 1461 1395 -66
Branches 299 279 -20
=========================================
- Hits 989 936 -53
+ Misses 468 456 -12
+ Partials 4 3 -1
Continue to review full report at Codecov.
|
|
Shorthand vs Children API examples render differently |
|
Thanks for the catch @miroslavstastny, it is fixed now. |
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md # src/components/Button/Button.tsx # src/themes/teams/components/Button/buttonStyles.ts
docs/src/examples/components/Button/Variations/ButtonExampleText.shorthand.tsx
Outdated
Show resolved
Hide resolved
kuzhelov
reviewed
Sep 6, 2018
| @@ -19,6 +19,11 @@ const Variations = () => ( | |||
| description="A button can be circular and formatted to show different levels of emphasis." | |||
| examplePath="components/Button/Variations/ButtonExampleEmphasisCircular" | |||
| /> | |||
| <ComponentExample | |||
| title="Text" | |||
| description="A button can be shown in form of a text to indicate some less-pronounced actions" | |||
There was a problem hiding this comment.
nit's nit: just for the sake of consistency, lets put dot at the end of description :)
There was a problem hiding this comment.
This is not a nit, is valid thing to be fixed as this is shown in our docs page :)
miroslavstastny
approved these changes
Sep 6, 2018
kuzhelov
reviewed
Sep 6, 2018
|
|
||
| return Icon.create(icon, { | ||
| defaultProps: { | ||
| xSpacing: !content ? 'none' : iconPosition === 'after' ? 'before' : 'after', | ||
| variables: { |
kuzhelov
approved these changes
Sep 6, 2018
|
Thanks for the updates @kuzhelov, will update the branch and merge it. |
added 2 commits
September 6, 2018 19:35
# Conflicts: # CHANGELOG.md # src/themes/teams/components/Button/buttonStyles.ts
-fixed console errors in the buttons examples
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Button
Adding text variant to the button, that will display the button without any background and border.
TODO
API Proposal
text: boolean
A button can be formatted to show only text in order to indicate some less-pronounced actions.
At this moment, the styles of the buttons containing the text prop, are just reducing the background and border, and on hover are altering the color of the text to be the
brandcolor. We may need to change this, as the current text color of the primary button is white, which is maybe not the most appropriate one. Once the API is approved, we can agree on colors used for the default, primary and secondary variations of the text button.