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

Making the customization option docs clearer about variable overrides #24033

Merged
merged 4 commits into from
Sep 27, 2017
Merged

Making the customization option docs clearer about variable overrides #24033

merged 4 commits into from
Sep 27, 2017

Conversation

Bobeta
Copy link
Contributor

@Bobeta Bobeta commented Sep 21, 2017

Current docs on the subject can easily be interpreted as the user should import their modified variables after importing bootstrap sass files, this PR aims to make it clearer #23963

@andresgalante
Copy link
Collaborator

andresgalante commented Sep 21, 2017

@Bobeta Thanks for sending this PR. It' be awesome if you could add a code snippet to show exactly how to load your variables before bootstrap's. It can be something silly like:

// Your variables overwrite first
$body-bg: #f00;

// Then import Bootstrap
@import "node_modules/bootstrap/scss/bootstrap";

or whatever you think it's best to drive the point better.

What do you think?

@Bobeta
Copy link
Contributor Author

Bobeta commented Sep 21, 2017

@andresgalante Sure mate, I baked in the examples from the next paragraph so that we don't repeat ourselves

$body-bg: $gray-900;
$body-color: $gray-600;
// Your variable overwrite first or a Sass file containing the modifications
$body-bg: #000000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd change this to #000 and #111 just to follow bootstrap coding guidelines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andresgalante You're right, did that with the new commit

@XhmikosR
Copy link
Member

@Johann-S: LGTM. Please merge if you agree too.

@Johann-S Johann-S merged commit 4cfe228 into twbs:v4-dev Sep 27, 2017
@mdo mdo mentioned this pull request Sep 27, 2017
mdo added a commit that referenced this pull request Sep 30, 2017
I believe #24033 is factually inaccurate, while also containing some
typos and grammar problems This PR addresses a couple things:

- Variable overrides can come before or after you import
Bootstrap—they'll still override the default value in our code.

- Removed the double `::` at the end of the paragraph.

- Fixed usage of `overwrite` when we mean `override`.

/cc @Johann-S @XhmikosR
mdo added a commit that referenced this pull request Sep 30, 2017
I believe #24033 is factually inaccurate, while also containing some typos and grammar problems. This PR addresses a few things:

- Variable overrides can come before or after you import Bootstrap—they'll still override the default value in our code.
- Removed the double  at the end of the paragraph.
- Fixed usage of  when we mean .

/cc @Johann-S @XhmikosR @Bobeta
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants