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

Generalize the adding bootstrap documentation #5631

Merged
merged 4 commits into from
Dec 24, 2018
Merged

Conversation

jquense
Copy link
Contributor

@jquense jquense commented Oct 30, 2018

Picking a particular react bootstrap library feels like too strong of an opinion for supplementary CRA docs. I trimmed it down to point out the two most popular options and remove library specific documentation to focus on the bits that are more CRA specific, e.g. using sass files.

@iansu
Copy link
Contributor

iansu commented Oct 30, 2018

There's also this PR that adds a section on installing plain bootstrap, in addition to reactstrap: #5565

@Timer
Copy link
Contributor

Timer commented Oct 31, 2018

We definitely don't want to advocate jQuery usage. I'd be OK with advocating something like "Bootstrap for styling only", where you use just CSS and no JS.

@jquense
Copy link
Contributor Author

jquense commented Oct 31, 2018

I think (at least it was my intention that) the copy I've added above here covers the "don't use jquery" aspect, with "here are the two main alternatives to that" but not trying to recommend one particular solution. w/d/t @Timer ?

@guledali
Copy link

guledali commented Nov 7, 2018

@jquense I actually like yours more than the reactstrap, that's because you have included an guide on how to theme which I think is really important using component library like react + a lot people are base their design system on bootstrap so the ability to easily customize the components is a win for me.

Example

bootstrapUtils.addStyle(Button, 'custom');

const customButtonStyle = (
  <div>
    <style type="text/css">
      {`
    .btn-custom {
        background-color: purple;
        color: white;
    }
    `}
    </style>
    <Button bsStyle="custom">Custom</Button>
  </div>
);

render(customButtonStyle);

What I don't like though is that your readme.md kinda looks messy to me, and I think it has to do with you maintaining two different bootstrap versions 3 and 4.
I wish your readme.md was as clean as the overall site https://react-bootstrap.netlify.com/

I made an issue on this check the react-bootstrap/react-bootstrap#3359

@jquense
Copy link
Contributor Author

jquense commented Nov 10, 2018

anything I can do here?

@@ -3,36 +3,28 @@ id: adding-bootstrap
title: Adding Bootstrap
---

You don’t have to use [reactstrap](https://reactstrap.github.io/) together with React but it is a popular library for integrating Bootstrap with React apps. If you need it, you can integrate it with Create React App by following these steps:
While you don’t have to use any specific library to integrate Bootstrap with React apps, it's often easier than trying to wrap the Bootstrap jQuery plugins. [React Bootstrap](https://react-bootstrap.netlify.com/) is the most popular option, that strives for complete parity with bootstrap. [reactstrap](https://reactstrap.github.io/) is also a good choice for projects looking for smaller builds at the expense of less features.
Copy link
Contributor

Choose a reason for hiding this comment

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

The end of this sentence doesn't sound right. expense of less featues sounds kind of like a double negative. Maybe something like at the expense of some features?

```

Import Bootstrap CSS and optionally Bootstrap theme CSS in the beginning of your `src/index.js` file:

```js
import 'bootstrap/dist/css/bootstrap.css';
import 'bootstrap/dist/css/bootstrap.css'
Copy link
Contributor

Choose a reason for hiding this comment

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

Our default template uses semicolons.

@@ -55,5 +47,5 @@ $body-bg: #000;
Finally, import the newly created `.scss` file instead of the default Bootstrap `.css` in the beginning of your `src/index.js` file, for example:

```javascript
import './custom.scss';
import './custom.scss'
Copy link
Contributor

Choose a reason for hiding this comment

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

Add semicolon back.

@iansu
Copy link
Contributor

iansu commented Nov 22, 2018

I think this is a good change. However, I agree with @Timer that it might be good to mention that just installing the bootstrap CSS is a valid option for theming only with no JS features (like dropdowns, etc).

@stale
Copy link

stale bot commented Dec 23, 2018

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 23, 2018
@stale stale bot removed the stale label Dec 24, 2018
@iansu iansu added this to the 2.1.3 milestone Dec 24, 2018
@iansu iansu merged commit e7b975e into facebook:master Dec 24, 2018
@iansu
Copy link
Contributor

iansu commented Dec 24, 2018

Thanks!

@lock lock bot locked and limited conversation to collaborators Jan 8, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants