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

make the spinner animation default to border #6468

Merged
merged 2 commits into from Nov 3, 2022

Conversation

factoidforrest
Copy link
Contributor

There's no good reason to always type out the animation for the spinner. There are only two choices, and the "border" option is the obvious choice as a default.

This should be a non-breaking change.

There's no good reason to always type out the animation for the spinner. There are only two choices, and the "border" option is the obvious choice as a default.

This should be a non-breaking change.
@factoidforrest
Copy link
Contributor Author

Im happy to add tests, just want to see if the core change here would be acceptable.

@kyletsang
Copy link
Member

Sounds like a good idea. For your code, you could add a default parameter to animation and keep the existing bsPrefix bit the same.

@factoidforrest
Copy link
Contributor Author

@kyletsang you're right, that's cleaner, I changed it. How does that look?

@kyletsang kyletsang merged commit fb3e4d5 into react-bootstrap:master Nov 3, 2022
@kyletsang
Copy link
Member

Thanks!

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

2 participants