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(Button): add close icon support (reactstrap/reactstrap#1182) #1206

Merged
merged 6 commits into from
Sep 25, 2018

Conversation

median-man
Copy link
Contributor

@median-man median-man commented Sep 8, 2018

This PR addresses #1182.

Add support for close icon utility from Bootstrap 4.
This commit adds an optional boolean property, 'close', to the
Button component. Applying the close prop will render the Close
icon utility from Bootstrap 4.

Update documentation on the Button component to demonstrate usage
of the close prop.

  • Bug fix
  • New feature
  • Chore
  • Breaking change
  • There is an open issue which this change addresses
  • I have read the CONTRIBUTING document.
  • My commits follow the Git Commit Guidelines
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This is my first time submitting a PR. I welcome any constructive feedback you have to offer if you are reviewing this PR.

Closes #1182

@TheSharpieOne
Copy link
Member

Sorry for the delay, but what do you think about defaulting children to <span aria-hidden>&times;</span> when close is true? That way people would be able to do <Button close /> to get what is probably the most common usages which includes ×. Providing children would of course show the children instead.

@median-man
Copy link
Contributor Author

median-man commented Sep 21, 2018

I think that is a great idea! I'll update the pr shortly.

Sorry for the delay, but what do you think about defaulting children to <span aria-hidden>&times;</span> when close is true? That way people would be able to do <Button close /> to get what is probably the most common usages which includes ×. Providing children would of course show the children instead.

@median-man median-man force-pushed the support-close-icon-util branch 2 times, most recently from ac32c58 to 9c66e7a Compare September 21, 2018 22:09
@median-man
Copy link
Contributor Author

median-man commented Sep 21, 2018

Pending checks, I've added the default children to the commit. If anything else needs fixing, I would be happy to see to it tomorrow. @TheSharpieOne

Add support for close icon utility from Bootstrap 4.
This commit adds an optional boolean property, 'close', to the
Button component. Applying the close prop will render the Close
icon utility from Bootstrap 4.
Update documentation on the Button component to demonstrate usage
of the close prop.

Closes reactstrap#1182
@TheSharpieOne TheSharpieOne merged commit 02f5e9a into reactstrap:master Sep 25, 2018
@TheSharpieOne
Copy link
Member

Thanks for the PR!

@median-man median-man deleted the support-close-icon-util branch September 25, 2018 03:30
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.

Feature Request - Support Close Icon Utility
2 participants