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

Add support for Spinners #3541

Merged
merged 2 commits into from Mar 14, 2019
Merged

Add support for Spinners #3541

merged 2 commits into from Mar 14, 2019

Conversation

@tjdavey
Copy link
Contributor

tjdavey commented Mar 14, 2019

Adds support for the spinners components introduced in Bootstrap 4.2.

Changes:

  • Adds new Spinner component
  • Add tests for new component
  • Creates new page in documentation for new component
  • Upgrades documentation CSS to Bootstrap 4.2 (given the claimed 4.2 support in the documentation itself)

Notes:

  • I wasn't sure about the terminology for the property that switches between the two styles of spinner component. I couldn't think of a good term that wouldn't overlap with an existing property. animation was chosen at it was the most representative of the changes the property made, but I feel it may still be too ambiguous. I'm open to alternative suggestions.
  • Whilst the documentation claims compatibility with Bootstrap 4.2 it was using a Bootstrap 4.1 CSS file. I've upgraded the documentation to a Bootstrap 4.2 version of the CSS file. I can remove this change and submit seperately if necessary.
…ion to Bootstrap 4.2 CSS.
Copy link
Member

taion left a comment

cool! can we add a type signature as well, for the TS users?

@tjdavey

This comment has been minimized.

Copy link
Contributor Author

tjdavey commented Mar 14, 2019

cool! can we add a type signature as well, for the TS users?

I'll add it now.

@taion

This comment has been minimized.

Copy link
Member

taion commented Mar 14, 2019

Thanks!

@jquense

This comment has been minimized.

Copy link
Member

jquense commented Mar 14, 2019

this is awesome

Copy link
Member

mxschmitt left a comment

Super duper fast, good Job!

@tjdavey

This comment has been minimized.

Copy link
Contributor Author

tjdavey commented Mar 14, 2019

@taion I've added the Typescript support. I don't have the deepest understanding of the way the Typescript definitions are structured for this project so it might be worth just checking the details of my implementation.

@taion
taion approved these changes Mar 14, 2019
@jquense jquense merged commit 2adb7c6 into react-bootstrap:master Mar 14, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@tjdavey tjdavey deleted the tjdavey:spinner-support branch Mar 14, 2019
@davecarlson

This comment has been minimized.

Copy link

davecarlson commented Mar 15, 2019

Any news when beta 6 will be released so we can use these without pulling in master ?

:)

@jquense

This comment has been minimized.

Copy link
Member

jquense commented Mar 15, 2019

soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.