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

Load Bootstrap and Fontawesome from vendor #13

Merged
merged 1 commit into from May 13, 2015

Conversation

gbrock
Copy link
Contributor

@gbrock gbrock commented May 13, 2015

Primarily, this fork removes a lot of resource files (Bootstrap and FontAwesome styles and fonts) in favor of loading them via composer.json.

As a bonus, this fork also allows users to supply their own Bootstrap and FontAwesome variables (frontend/variable-overrides).

Perhaps overly-opinionated, this fork also removes the HTML5 Boilerplate CSS defaults (previously found in frontend/main) in favor of the defaults provided by Bootstrap. This has the (perhaps desirable) side-effect of drastically shortening the main stylesheets.

Overall, this fork is designed to clean up the process of rendering vendor libraries and user stylesheets, both LESS and SASS, and to put package control in charge of keeping these libraries up-to-date.

Potential Issues:

  • The backend styles remain largely untouched as it seems they are still under more serious development (i.e. backend/main.scss).
  • JS files should not have been affected by these changes. It might be prudent to also use the package-controlled Bootstrap JS.

- Added Bootstrap and FontAwesome to composer.json (“vendor includes”)
- Altered frontend/main stylesheet to load user variables and vendor
includes (both LESS and SASS)
- gulpfile.js now copies vendor-included webfont files to /public
directory
- Commented gulpfile.js
- Removed vendor-specific rendering from gulpfile.js
- Removed /resources/assets/less/bootstrap (defer to vendor includes)
- Removed /resources/assets/less/font-awesome (defer to vendor includes)
- Removed /resources/assets/sass/bootstrap (defer to vendor includes)
- Removed /resources/assets/sass/font-awesome (defer to vendor includes)
- Removed H5BP from frontend/main stylesheet (redundant w/ Bootstrap)
@rappasoft
Copy link
Owner

Wow good work. Let me go over it, it seems like a better solutions so as soon as I understand it all I don't see a reason why it can't be merged.

@rappasoft
Copy link
Owner

Also, your first issue: backend/main.scss is a compiled version of the LESS for the AdminLTE theme, since it only comes in LESS, I wasn't going to take the time to convert it to SCSS. So it is not under development by me anyway, I figured most people would swap out the theme for their own anyway.

@gbrock
Copy link
Contributor Author

gbrock commented May 13, 2015

Thanks!

re: backend/main.scss: Understood.

@rappasoft rappasoft self-assigned this May 13, 2015
@rappasoft
Copy link
Owner

I like this it just needs some tweaks:

  1. The compiler is bitching at me unless I put the font import url in quotes in backend/main.scss
  2. Even after that for some reason the font weights are changing on the frontend between the 2 versions. See attached.

2015-05-13 09 31 34 am
2015-05-13 09 31 27 am

@rappasoft
Copy link
Owner

I'm gonna try to merge and fix it if you don't mind.

rappasoft added a commit that referenced this pull request May 13, 2015
Load Bootstrap and Fontawesome from vendor
@rappasoft rappasoft merged commit cbfccb2 into rappasoft:master May 13, 2015
@rappasoft
Copy link
Owner

Merged. Pull it and see if it's what you intended.

@gbrock
Copy link
Contributor Author

gbrock commented May 13, 2015

Everything seems to be working great, except for variable overrides in the SASS version. The overrides need to come before the variables they'll override, unlike in LESS, where they must be declared later. This also takes care of the font weight issue you mentioned: it should override the font as intended when loaded like so (assets/sass/frontend/main.scss):

// Any Bootstrap or FontAwesome-specific variables added here will overwrite the defaults.
@import "variable-overrides";

@import '../../../../vendor/twbs/bootstrap-sass/assets/stylesheets/_bootstrap';
@import '../../../../vendor/fortawesome/font-awesome/scss/font-awesome';


// Front-end SCSS goes here.

...while the LESS version can remain as-is:

@import '../../../../vendor/twbs/bootstrap/less/bootstrap';
@import '../../../../vendor/fortawesome/font-awesome/less/font-awesome';

// Any Bootstrap or FontAwesome-specific variables added here will overwrite the defaults.
@import 'variable-overrides';

// Front-end LESS goes here.

@rappasoft
Copy link
Owner

Ok fixed, still weight issue though.

@gbrock
Copy link
Contributor Author

gbrock commented May 13, 2015

Hmm... Doesn't look like the /public assets have changed at all? Gulp will need to re-render the SCSS files with the correct variables (in this case, $font-family-sans-serif was being correct applied through LESS but not through SASS).

@gbrock
Copy link
Contributor Author

gbrock commented May 13, 2015

Just double-checked after pulling the latest changes: rendering either SASS or LESS results in the same font/weight for me, so it seems good as far as I can tell.

I'm a little green with git; if there's anything more specific I can do to help troubleshoot this further please let me know.

@rappasoft
Copy link
Owner

I'm probably just as green. It's fine by me.

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