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

Conversation

tjdavey
Copy link

@tjdavey 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.

Copy link
Member

@taion taion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@tjdavey
Copy link
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
Copy link
Member

taion commented Mar 14, 2019

Thanks!

@jquense
Copy link
Member

jquense commented Mar 14, 2019

this is awesome

mxschmitt
mxschmitt previously approved these changes Mar 14, 2019
Copy link
Member

@mxschmitt mxschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super duper fast, good Job!

@tjdavey
Copy link
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.

@jquense jquense merged commit 2adb7c6 into react-bootstrap:master Mar 14, 2019
@tjdavey tjdavey deleted the spinner-support branch March 14, 2019 15:49
@davecarlson
Copy link

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

:)

@jquense
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants