Added --inlineImages option to embed images. #32
Conversation
|
this is so amazing. I'm going to try to review this tomorrow morning, but it looks great! Thanks :) |
|
Cool I think the only change will be to disable it by default that way you'll need to supply the option to have the images embeded. No rush I can use it in my project as is :) Sent from my iPhone On 24 Jun 2012, at 04:24, Jacob Thorntonreply@reply.github.com wrote:
|
| var props = toCSS.apply(this, arguments) | ||
|
|
||
| // do we have a url here? | ||
| if (/url\(([a-zA-Z\"\'.]+)\)/.test(props)) { |
There was a problem hiding this comment.
maybe let's just simplify this to: /url\(/.test(props) or something along those lines.
There was a problem hiding this comment.
Good call, brought up a bug where it wasn't embedding if images links included paths (../sprite.png etc). Have added test for this now.
Did bring up another issue where the tests were failing because the relaxed regex test was picking up font-face src in blog.css and there is no file to embed. I just removed the @font-face declaration from blog.css hope that's ok?
|
few little things, but mostly looks good - thanks again! |
|
Cool looks good I will make those changes on the weekend probably. |
… added another test for embedding images that include paths.
|
@fat any ideas if/when this will get merged into master? |
|
https://github.com/GoalSmashers/enhance-css i really like how they do their inline images.. really flexible to just add ?embed after the images you want it to base64. also they have an option to spit out a non embeded version for pre ie8 support.. |
|
Looks good. Not something I will need. I have about 150-200k worth on encoded images, defined in one file separate from the main stylesheet. Anything I need to encode I add to this stylesheet and my build script enables the inlineImages option only on this stylesheet (using recess) and it copies the original to a built folder (to serve to ie8). This is a benefit for two reasons a) any css changes don't cause the user to download unchanged encoded images. My sprites rarely change my css changes a lot. Second I async the stylesheet in at the bottom of the page with some js. Standard css link's in the head cause most browsers to block rendering until complete. Seeing as the sprites are non-critical this makes sense. Enhance looks good, there would be no downsides in using it, but there is also no real benefit for my specific use case :) |
Added --inlineImages option to embed images.
|
sorry @gcoop got a bit caught up with other things. The changes all look good - just pushed out a new version 1.1.0 which has this in it :) |
|
btw readme needs to be updated to reflect this option |
|
if i do |
Have enabled it by default, which may not be suitable.
I did have to make one change to core.js so
.onwould have access to the file path that was being processed, because I needed to get the image data relative to the .css file. I couldn't think of another way to do it.Added lint rule and tests for both compile and linting.