Skip to content

feat(outline): Make outline a separate prop#62

Merged
eddywashere merged 2 commits intoreactstrap:masterfrom
TheSharpieOne:feature/outline-prop
Jul 28, 2016
Merged

feat(outline): Make outline a separate prop#62
eddywashere merged 2 commits intoreactstrap:masterfrom
TheSharpieOne:feature/outline-prop

Conversation

@TheSharpieOne
Copy link
Copy Markdown
Member

The outline for buttons and cards is now it's own props separate from the color value.
Based on #33 (comment)

This also adds additional tests around this new props.

Also:
Cards, unlike buttons, do not have a default color. Thus adding outline prop to a card without a color will do nothing.
Is this acceptable? Should it produce a warning/error? Should it default to 'card-outline-secondary' when outline is provided but no color is provided?

The outline for buttons and cards is now it's own props separate from the color value.
Based on reactstrap#33 (comment)
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 28, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling ee5fa1e on TheSharpieOne:feature/outline-prop into f617dc5 on reactstrap:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 28, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 7496c91 on TheSharpieOne:feature/outline-prop into b62ee01 on reactstrap:master.

@eddywashere
Copy link
Copy Markdown
Member

@TheSharpieOne thanks a ton for the prs!

I think this is acceptable behavior. I think card already has a solid style without background variants applied where as the default .btn kind of looks weird and isn't really shown in the docs.

re: warnings/errors. I'd love to throw warnings in dev mode that catch issues like this and not include those warnings in production builds.

@eddywashere eddywashere merged commit c65e952 into reactstrap:master Jul 28, 2016
@TheSharpieOne TheSharpieOne deleted the feature/outline-prop branch August 17, 2016 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants