-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Update bootstrap.rst #9918
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
Update bootstrap.rst #9918
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Eoras thanks for this contribution! It contains some fixes that we definitely must do ... but also some changes that should be reverted for the reasons I gave in the inlined comments.
Thanks!
frontend/encore/bootstrap.rst
Outdated
@@ -1,18 +1,24 @@ | |||
Using Bootstrap CSS & JS | |||
======================== | |||
Using Bootstrap 4 CSS & JS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert this change and remove the 4
version number. This article will be perpetually updated when new versions of Bootstrap are published, so there's no need to update the title.
frontend/encore/bootstrap.rst
Outdated
|
||
Want to use Bootstrap (or something similar) in your project? No problem! | ||
First, install it. To be able to customize things further, we'll install | ||
``bootstrap-sass``: | ||
``bootstrap`` and popper.js: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert this change because the article is about Bootstrap, so we don't want to install other libraries. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I delete popper.js but bootstrap 4 need it to work with tooltips. (Yarn and nmp say to add this dependancie when we install bootstrap so is ok)
frontend/encore/bootstrap.rst
Outdated
|
||
Importing Bootstrap Sass | ||
or if you prefer to use npm: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to use yarn
in the Symfony Docs. This is of course a decision that can be debated and changed. But meanwhile we can't change individual articles. So, let's remove this change about "npm" and the other one later in this article.
file into ``global.scss``. You can even customize the Bootstrap variables first! | ||
|
||
.. tip:: | ||
|
||
If you don't need *all* of Bootstrap's features, you can include specific files | ||
in the ``bootstrap`` directory instead - e.g. ``~bootstrap-sass/assets/stylesheets/bootstrap/alerts``. | ||
|
||
After including ``bootstrap-sass``, your Webpack builds might become slow. To fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just asking, why did you remove this explanation about resolveUrlLoader
? Is it no longer needed in Bootstrap 4? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I delete this because now on the scss file from bootstrap, we don't need to change the path from the webfont etc, so WebPack work realy fine now... i'm doing some test but for me isn't necessary anymore.
Update after comments
This PR was submitted for the 4.1 branch but it was squashed and merged into the 3.4 branch instead (closes #9918). Discussion ---------- Update bootstrap.rst <!-- If your pull request fixes a BUG, use the oldest maintained branch that contains the bug (see https://symfony.com/roadmap for the list of maintained branches). If your pull request documents a NEW FEATURE, use the same Symfony branch where the feature was introduced (and `master` for features of unreleased versions). --> Commits ------- 38a5435 Update bootstrap.rst
This is now merged! We merged in 3.4 and we'll merge in the other branches automatically. @Eoras thanks for your first Symfony Docs contribution! |
@@ -1,18 +1,18 @@ | |||
Using Bootstrap CSS & JS | |||
======================== | |||
============================ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be reverted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing Bootstrap Sass | ||
$ yarn add bootstrap --dev | ||
Importing Bootstrap 4 | ||
------------------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be shortened.
|
||
Importing Bootstrap JavaScript | ||
------------------------------ | ||
--------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be reverted.
No description provided.