Include Normalize.css if Bootstrap is not present #269
Conversation
| @@ -11,7 +11,8 @@ | |||
| <!-- Place favicon.ico and apple-touch-icon.png in the root directory --> | |||
| <!-- build:css styles/vendor.css --> | |||
| <!-- bower:css --> | |||
| <% if (includeBootstrap && !includeCompass) { %><link rel="stylesheet" href="bower_components/bootstrap/dist/css/bootstrap.min.css"><% } %> | |||
| <% if (includeBootstrap && !includeCompass) { %><link rel="stylesheet" href="bower_components/bootstrap/dist/css/bootstrap.min.css"><% } | |||
| else if (!includeBootstrap) { %><link rel="stylesheet" href="bower_components/normalize-css/normalize.css" /><% } %> | |||
There was a problem hiding this comment.
@eddiemonge how do I write this according to your standards, i.e. closing the tag on the same line where I'm opening it? Is the only solution writing everything on one line?
|
I would say we can exclude Normalize from the description. |
|
Bootstrap itself ships with Normalize, so including Normalize again would be redundant and would increase the stylesheet's file size. To clarify:
|
|
Is it possible to drop all references to vendor.css being included at all for the last case? |
|
Yeah, it is. If nobody has a problem with that. |
|
Done. I tested it (manually) and it works correctly in each combination. |
|
you are missing bower:css for includebootstrap and compass. bower:css should exist for all options |
|
Ok, but I'm not happy with this solution. I want to include something when Compass and Bootstrap are present. Can't I just include an empty CSS file to avoid the bad request? |
|
I think we should just include the empty CSS file if it'll avoid any further exceptions being thrown. Does anyone forsee issues with doing that? Would love to finally nail down a solution for this that we can land. It keeps getting reported. |
|
Isn't that an issue with grunt-usemin? Shouldn't it not try to load an asset that doesn't exist? Or should it create an empty one instead? |
|
I don't think it should try loading an asset that doesn't exist. At the same time, given the slowdown in usemin's release cycle I wonder if we can come to an agreement about a way if fixing this which either involves a workaround factoring in the above discussion or a minor patch to usemin. |
|
A minor patch to usemin would solve this problem more efficiently. |
I included it instead of Bootstrap because Bootstrap already ships with Normalize. What do we do in the case when Sass & Compass are also present? Because then both Bootstrap and Normalize aren't included, which leaves
vendor.cssempty again. 😟Also, do I add Normalize to the description
Out of the box I include...or is it unnecessary because Normalize is part of H5BP anyway?This should fix #242 once it's resolved.