Skip to content
This repository has been archived by the owner on Mar 18, 2022. It is now read-only.

Upload images referenced in CSS files #18

Merged
merged 4 commits into from
Sep 19, 2012
Merged

Conversation

JoshuaGross
Copy link
Contributor

I believe this was requested - though it's not through Stylus, LESS, etc. I decided it was easier to just parse out any images referenced in the CSS file. For me it was useless to CDNify CSS at all if it didn't also upload all images.

Note that this works with the background-image, background, and contents CSS attributes. The image paths MUST be absolute (e.g. if you do a "../whatever.png" it won't work) and a descendent of options.publicDir.

Let me know what you think!

@JoshuaGross JoshuaGross mentioned this pull request Sep 18, 2012
@jmonster
Copy link

F YEAH :)

@niftylettuce
Copy link
Collaborator

cool let me check this out :)

@niftylettuce niftylettuce merged this pull request into ladjs:master Sep 19, 2012
@niftylettuce
Copy link
Collaborator

@JoshuaGross I think you need to swap imageResource with url?

@niftylettuce
Copy link
Collaborator

Rolled back

@JoshuaGross
Copy link
Contributor Author

Not sure what you mean. Twitter bootstrap/all the CSS I've ever written looks like background: url(...)

-- Joshua Gross

On Sep 19, 2012, at 1:22 PM, Nick Baugh notifications@github.com wrote:

@JoshuaGross I think you need to swap imageResource with url?


Reply to this email directly or view it on GitHub.

@niftylettuce
Copy link
Collaborator

U defined imageResource car but didn't use it?
On Sep 19, 2012 2:57 PM, "Joshua Gross" notifications@github.com wrote:

Not sure what you mean. Twitter bootstrap/all the CSS I've ever written
looks like background: url(...)

-- Joshua Gross

On Sep 19, 2012, at 1:22 PM, Nick Baugh notifications@github.com wrote:

@JoshuaGross I think you need to swap imageResource with url?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-8702325.

@JoshuaGross
Copy link
Contributor Author

Sorry, dumb response on my part. I assumed you were taking about CSS, not my JS code. I won't be able to look properly until tomorrow, do you have a fix that works?

-- Joshua Gross

On Sep 19, 2012, at 1:58 PM, Nick Baugh notifications@github.com wrote:

U defined imageResource car but didn't use it?
On Sep 19, 2012 2:57 PM, "Joshua Gross" notifications@github.com wrote:

Not sure what you mean. Twitter bootstrap/all the CSS I've ever written
looks like background: url(...)

-- Joshua Gross

On Sep 19, 2012, at 1:22 PM, Nick Baugh notifications@github.com wrote:

@JoshuaGross I think you need to swap imageResource with url?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-8702325.


Reply to this email directly or view it on GitHub.

@niftylettuce
Copy link
Collaborator

*var not car lol

@niftylettuce
Copy link
Collaborator

ok i see what you did with imageResource variable -- need to look into if you appended timestamp etc

@niftylettuce
Copy link
Collaborator

@JoshuaGross this works great man, awesome work. I published it in version 0.0.6 👍

@niftylettuce
Copy link
Collaborator

@jmonster you can use this in version 0.0.6 :feelsgood:

@niftylettuce
Copy link
Collaborator

@JoshuaGross one additional thing -- is that we should modify the CSS to support the appended timestamp format (e.g. background-image: url('/img/joshua.png') -> background-image: url('//abc123.cloudfront.net/img/joshua.123456789.png)`) -- do you think you can build that in too?

@niftylettuce
Copy link
Collaborator

edit: I updated previous comment from /img/joshua.png to /img/joshua.{{mtime}}.png

@JoshuaGross
Copy link
Contributor Author

Awesome!

Wrt to including the timestamp - not actually sure how I would do that. The imageResource returned is undefined currently...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants