bguiz / broccoli-sprite

BroccoliJs plugin that generates CSS image sprites
http://blog.bguiz.com/post/88719320815/broccoli-sprite-released/
16 stars 7 forks source link

Swap out the sprite library? #5

Open toranb opened 9 years ago

toranb commented 9 years ago

I'm migrating from a gulp based ember build to something ember-cli based and it looks like this is the library to start with. I'd like to take a run at it using the library shown below but I wanted to see if it meets your needs (ie- does SVG w/ a png fallback work or does this need something further) ?

Thanks for the amazing amount of ember-cli OSS work you do! Love this project !

https://github.com/jkphl/svg-sprite/blob/master/lib/svg-sprite.js#L80

bguiz commented 9 years ago

I'm migrating from a gulp based ember build to something ember-cli based and it looks like this is the library to start with.

Thanks for the amazing amount of ember-cli OSS work you do! Love this project !

Thank you very much!

I'd like to take a run at it using the library shown below but I wanted to see if it meets your needs (ie- does SVG w/ a png fallback work or does this need something further) ?

You are now the second person to request support for SVG sprite sheets. I have been meaning to add that for a while, but have yet to find the time to do so.

https://github.com/jkphl/svg-sprite/blob/master/lib/svg-sprite.js#L80

It appears that this sprite library might not be able to meet the needs, for a couple of reasons:

  1. It only supports SVG images (a library that supports both raster and vector images would be ideal)
  2. The structure of the options required is incompatible

Both of these can be worked around with some difficulty:

  1. Only SVG --> Use both node-sprite-generator and svg-sprite, and pass the relevant images to each one
  2. Options structure incompatibility --> Invent a options structure particular to broccoli-sprite, which gets transformed accordingly for each of node-sprite-generator and svg-sprite

However, these would introduce much tight coupling, leading to broccoli-sprite, and therefore also ember-sprite, getting broken when node-sprite-generator and svg-sprite are updated. A much more ferasible option perhaps, would be for ember-sprite to take in two top level configurations, for sprite (the one that it does already), and vectorSprite, which will conform to what is required by svg-sprite. The declaration in Brocfile.js in an ember-cli app should then take this structure:

var app = new EmberApp({
    /* other options */
    sprite: { /* raster image sprite options, to be passed to

node-sprite-generator via broccoli-sprite / }, vectorSprite: { / vector image (SVG) sprite options, to be passed to svg-sprite via broccoli-sprite */ }, }});

Does this work for you?

W: http://bguiz.com

On 9 December 2014 at 13:39, Toran Billups notifications@github.com wrote:

I'm migrating from a gulp based ember build to something ember-cli based and it looks like this is the library to start with. I'd like to take a run at it using the library shown below but I wanted to see if it meets your needs (ie- does SVG w/ a png fallback work or does this need something further) ?

Thanks for the amazing amount of ember-cli OSS work you do! Love this project !

https://github.com/jkphl/svg-sprite/blob/master/lib/svg-sprite.js#L80

— Reply to this email directly or view it on GitHub https://github.com/bguiz/broccoli-sprite/issues/5.

toranb commented 9 years ago

Thanks for the blazing fast reply/work ethic to keep this project moving forward :)

On the svg library - totally understand the issue. I was hoping to help provide a solution but didn't take into account the needs you mentioned.

Does it make sense to include SVG in this (assuming that will require an additional node-library) or should someone create a standalone SVG sprite library?

If this is the place for SVG support (and your time is limited) what can the community do to get this pulled in?

Again -thank you, thank you, thank you!

bguiz commented 9 years ago

Thanks for the blazing fast reply/work ethic to keep this project moving forward :)

Thanks ... if only I had more bandwidth!

If this is the place for SVG support (and your time is limited) what can the community do to get this pulled in?

Yes, I would say that broccoli-sprite is the right place to support SVGs. The way I'm thinking that this would work would be to for index.js to be replicated in broccoli-sprite.

In the replica file, say vector.js, we do the same thing as index.js, except that we invoke svg-sprite instead of node-sprite-generator.

Subsequently, in ember-sprite, the change would be minimal.

That way, we keep it modular, the functionality for the two different underlying image processing libraries are located in two separate files.

I'll give this some more thought later on in the day. W: http://bguiz.com

On 10 December 2014 at 09:18, Toran Billups notifications@github.com wrote:

Thanks for the blazing fast reply/work ethic to keep this project moving forward :)

On the svg library - totally understand the issue. I was hoping to help provide a solution but didn't take into account the needs you mentioned.

Does it make sense to include SVG in this (assuming that will require an additional node-library) or should someone create a standalone SVG sprite library?

If this is the place for SVG support (and your time is limited) what can the community do to get this pulled in?

Again -thank you, thank you, thank you!

— Reply to this email directly or view it on GitHub.

bguiz commented 9 years ago

I am going to spike this on a new branches in both ember-sprite and broccoli-sprite shortly.

bguiz commented 9 years ago

Here it is:

Completely untested at the moment, but I figure it should be a good starting point.

Some work that definitely needs to be done, which I have left as a TODO is at https://github.com/bguiz/broccoli-sprite/blob/feature/svg/vector.js#L44

toranb commented 9 years ago

Wow that was fast! So this is on a feature branch today - if so how can I get this by installing ember-spite or do I need to write my own ember-cli wrapper until you get this into master? Or am I way off?

update I just noticed you had the same feature branch for both projects :)

Thanks again for the awesome work!

bguiz commented 9 years ago

So this is on a feature branch today - if so how can I get this by installing ember-spite or do I need to write my own ember-cli wrapper until you get this into master?

Just in case you are not familiar (I don't intend be patronising or anything), the way ember-cli works is that it looks in your app's package.json and iterates over all dependencies defined. For each one, it looks in it's package.json, and checks for the presence of an ember-addon tag, and if present attempts to load it. So all you really need to do is have the branch copy of ember-sprite in your app's node_modules folder.

The easiest way to do this is using npm link.

cd.. #from yourapp
git clone (git URL for ember-sprite)
cd ember-sprite
git checkout svg # switch branches
cd ../yourapp
npm link  ../ember-sprite

Then do the same set of steps, this time for ember-sprite to link to broccoli-sprite. npm link is essentially a symlink, so it should be pretty straightforward to get how it gets wired up - simply do ls -la node_modules in your app folder.

This is the setup that I use while developing, so that whatever copies I have currently checked out or am in the midst of coding are the ones used.

update I just noticed you had the same feature branch for both projects :)

Yes that is right, supporting SVG sprites would require a change that spans both projects. When I created the projects, I considered having just one, but I followed the lead of ember-addons created by @stefanpenner and @rwjblue on this front.

My projects only need to sprite raster images, so I do not have a use case handy. More importantly, someone who needs to use it in a real app will have a much better idea of the requirements and specs that need to be fulfilled. Thus, I think I would like to hand these branches over to you (or anyone else interested in SVG sprites) at this stage. Are you keen to take it from here?

@williamsbdev - I think you might be interested to follow this thread after this one: https://github.com/bguiz/ember-sprite/issues/6

On 12 Dec 2014 02:41, "Toran Billups" notifications@github.com wrote:

Wow that was fast! So this is on a feature branch today - if so how can I get this by installing ember-spite or do I need to write my own ember-cli wrapper until you get this into master? Or am I way off?

Thanks again for the awesome work!

— Reply to this email directly or view it on GitHub https://github.com/bguiz/broccoli-sprite/issues/5#issuecomment-66637147.

JarrodCTaylor commented 9 years ago

Just wanted to chime in and say I would also be interested in SVG sprit sheets. :+1:

toranb commented 9 years ago

@bguiz excellent work! I'll take a run at it locally as you described and provide some feedback. Honestly I'm looking for the following (from a SVG feature perspective)

1) point the plugin at a directory of all svg images 2) it will concat all those svg images into a single svg sprite image (and this single sprite will be something I can access from the public images directory) 3) the css that is related to this new sprite will be included in the final output (assets/project.css or whatever css file name we end up with in ember-cli)

I assume this project can do that much today (because it could do that much before the SVG requirement). Correct me if I'm wrong :)

bguiz commented 9 years ago

Yes that is pretty much exactly what the raster spriting works. Let me know how it goes with svg spriting! On 12 Dec 2014 15:01, "Toran Billups" notifications@github.com wrote:

@bguiz https://github.com/bguiz excellent work! I'll take a run at it locally as you described and provide some feedback. Honestly I'm looking for the following (from a SVG feature perspective)

1) point the plugin at a directory of all svg images 2) it will concat all those svg images into a single svg sprite image (and this single sprite will be something I can access from the public images directory) 3) the css that is related to this new sprite will be included in the final output (assets/project.css or whatever css file name we end up with in ember-cli)

I assume this project can do that much today (because it could do that much before the SVG requirement). Correct me if I'm wrong :)

— Reply to this email directly or view it on GitHub https://github.com/bguiz/broccoli-sprite/issues/5#issuecomment-66729679.

toranb commented 9 years ago

The first issue I ran up against is that ember-sprite doesn't have access to broccoli-sprite/vector (from the feature branch of ember-sprite).

var brocVectorSprite = require('broccoli-sprite/vector');

I did an npm install of ember-sprite like so ... but not dice

"ember-sprite": "git+https://github.com/bguiz/ember-sprite#feature/svg",
bguiz commented 9 years ago

Yes, I would expect that. You need to do the npm link thing twice, once from ember-sprite to point to broccoli-sprite and another to point from your app to ember-sprite. The package.json version of any lib gets ignored when that lib has been npm linked. On 13 Dec 2014 01:17, "Toran Billups" notifications@github.com wrote:

The first issue I ran up against is that ember-sprite doesn't have access to broccoli-sprite/vector (from the feature branch of ember-sprite).

var brocVectorSprite = require('broccoli-sprite/vector');

I did an npm install of ember-sprite like so ... but not dice

"ember-sprite": "git+https://github.com/bguiz/ember-sprite#feature/svg",

— Reply to this email directly or view it on GitHub https://github.com/bguiz/broccoli-sprite/issues/5#issuecomment-66777460.

bguiz commented 9 years ago

How did you go with this? I'll spell out the steps explicitly, for your convenience:

Let DIR be the directory where you you have your app's folder. Such that ${DIR}/yourapp/package.json is where your app's package.json file is located.

# npm link yourapp to ember-sprite
cd ${DIR}
git clone git@github.com:bguiz/ember-sprite.git
cd ember-sprite
git checkout svg # switch branches
cd ${DIR}/yourapp
npm link  ../ember-sprite

# npm link ember-sprite to broccoli-sprite
cd ${DIR} # from ember-sprite
git clone git@github.com:bguiz/broccoli-sprite.git
cd broccoli-sprite
git checkout scg #switch branches
cd ${DIR}/ember-sprite
npm link ../broccoli-sprite
toranb commented 9 years ago

Awesome step by step! I've been out of town all weekend but I'll take a run at it again tonight!

toranb commented 9 years ago

No dice on the complex npm linking locally - to avoid the branch/npm pain could I just pass along an example to see if you can get it to work? I assume this works as the commits look like they simply delegate out to the underlying vector lib (for the more complex svg work).

I want to use it the same way you do for png images today

1) I'll have a directory of 2 svg images

https://github.com/toranb/example-cli-with-phantomjs/tree/master/images

2) I want the addon to look at these and concat them into a single sprite with whatever name (configurable like you have it now is cool). Just so long as this is drop'd into the dist directory so I can fetch it via http as I'm sure you do today w/ png based sprites.

3) this should generate a css file to go along with it and you can add the prefix icon (similar to how the png images are prefixed in the stylesheet that gets generated).

4) the above generated css should be included/ concat'd so that my ember server command loads it up

A dead simple example app (what I was trying to do) should be no problem 15 minutes tops but the npm setup is killing me.

If you are just curious how this will work/should work I hope the above explains it. If you get it working and just want to publish a release that is beta ish to help me flush it out on a real project that would work for me.

Let me know if this makes sense / sorry I couldn't get the npm linking to work this time around

bguiz commented 9 years ago

No dice on the complex npm linking locally

If you post your exact error message(s), that would be really helpful. It could quite easily be an oversioght of a really trivial thing.

to avoid the branch/npm pain could I just pass along an example to see if you can get it to work? I assume this works as the commits look like they simply delegate out to the underlying vector lib (for the more complex svg work).

Yes that is correct, these build tools simply provide a broccoli and ember-cli friendly wrapper around the underlying functionality.

No promises, but I'll find some time to take a look at this over the weekend.

To expedite this, I'll need something from you. For the project that you have linked to, example-cli-with-phantomjs, can you please supply the exact usage you want to invoke svg-sprite with. Referring to "Node.js module usage", let me know what the parameters to SVGSprite.createSprite( /* ... */ ); should be.

Ideally, the tool should output what you have described in steps 1 through 3 in your list of four requirements. The 4th step is the responsibility of broccoli-sprite and ember-sprite, so don't worry about that.

but the npm setup is killing me.

I have yet to experience any problems with npm link, so I am still curious as to what is preventing you from getting it going. A simple diagnostic would be to inspect the node_modules folder in your app, and in ember-sprite.

Say your app is located in /code/node/yourapp:

$ ls -la /code/node/yourapp/node_modules
# one of the lines should contain:
# ember-sprite -> /code/node/ember-sprite
$ ls -la /code/node/yourapp/ember-sprite
# one of the lines should contain:
# broccoli-sprite -> /code/node/broccoli-sprite

If the symbolic links do not point to the correct location, as shown above, then npm link has not been done correctly. If the symbolic links are indeed pointing to the correct locatiomn, then the next think to check is whether the locations they are pointing to, are indeed the git repositories that you have cloned, and that they are in the right branch.

toranb commented 9 years ago

I'd certainly be up to share a tmux session and hack through it if you have the patience to mentor a newb like myself (if an option ping me -> toranb at gmail) **no obligation

About the options I'd like - this is what I would put into the Brocfile if I had this project/infinite time to get it right

var app = new EmberApp({
    sprite: {
      src: [
        'images/**/*.svg'
      ],
      spritePath: 'assets/sprites.svg',
      stylesheetPath: 'assets/sprites.css',
      stylesheetOptions: {
        prefix: 'icon-',
        spritePath: '/assets/sprites.svg'
      },
    },
}});

This is based on my experience w/ the following gulp plugin

https://github.com/shakyshane/gulp-svg-sprites

In that plugin I would simply set the src for my svg images and it would dump a css file + sprite.svg when completed (sadly no real caching so my build times took a hit). Thanks again for all the help!

bguiz commented 9 years ago

About the options I'd like - this is what I would put into the Brocfile if I had this project/infinite time to get it right

I do not mean the options that you would like to configure in your Brocfile. I mean the options that you would call svg-sprite directly with. i.e. in the following line:

 SVGSprite.createSprite( /* ... */ );

what would the parameters that you would pass in be? Ignore any thing to do with ember-cli, broccoli or these projects. Just what you would do if you were to call this function using just NodeJs + svg-sprite, and nothing else.

Regarding npm link errors: So I tried out the project that you linked above: https://github.com/toranb/example-cli-with-phantomjs and the linking worked all fine, but I did get this error message:

ENOENT, no such file or directory 'node_modules/ember-cli-ic-ajax/node_modules/'

So I did npm install --save ember-cli-ic-ajax, and that went away. Was that the problem that you encountered?

toranb commented 9 years ago

After 9 replies I finally get it :)

Here is the sample I threw together that will dump the css and sprite svg to the dist/assets folder (for ember-cli) using the icon prefix

https://github.com/toranb/svg-hacking-with-node

TL;DR

var SVGSprite = require('svg-sprite');
var options = {
render: {
css: 'sprite.css'
},
prefix: 'icon',
spritedir: 'images/svg'
};
var callback = function(err, results) {
//todo: throw error
};
SVGSprite.createSprite('images', 'dist/assets', options, callback);
toranb commented 9 years ago

It would be nice to proxy the prefix and spritedir options for the first iteration as you will abstract away the sprite.css concat step (making that config option much less important).

The other option worth adding is the "images" directory used to find the svg files to concat. Some devs use public but in this case I'd like to keep my options open (as public images/other assets get copied into dist/assets and this addon will do that for us -only pushing a single svg into dist/assets when it's done)

toranb commented 9 years ago

Did the spike I threw together show what you needed or are you looking for more information yet to start adding this feature to the project? Thanks again for all the help bringing this addon to the masses!

bguiz commented 9 years ago

OK, I have have just pushed some changes to the spike branch on broccoli-spite: feature/svg

It doesn't work yet, very much a WIP, but it should give you an idea of where I am with this at the moment. W: http://bguiz.com

On 22 December 2014 at 05:28, Toran Billups notifications@github.com wrote:

Did the spike I threw together show what you needed or are you looking for more information yet to start adding this feature to the project? Thanks again for all the help bringing this addon to the masses!

— Reply to this email directly or view it on GitHub.

toranb commented 9 years ago

Wow thanks for working on this like a boss ! And if I haven't said it already - it's awesome projects like this that help me iterate so quickly with ember! thank you, thank you, thank you!

bguiz commented 9 years ago

OK, another push has gone in, this time to both broccoli-sprite and ember-sprite, in their respective feature/svg branches.

At the moment, it successfully runs the SVG sprite tool, generating the required CSS and SVG files. However, merging it back into the main app's broccoli tree still does not work (I have been trying to debug that in vain for the past half an hour). I'm going to have to leave it as that for the moment.

If you - or anyone else here - would like to have a play and figure out where it is going wrong, go for it!

W: http://bguiz.com

On 23 December 2014 at 15:12, Toran Billups notifications@github.com wrote:

Wow thanks for working on this like a boss ! And if I haven't said it already - it's awesome projects like this that help me iterate so quickly with ember! thank you, thank you, thank you!

— Reply to this email directly or view it on GitHub https://github.com/bguiz/broccoli-sprite/issues/5#issuecomment-67920351.

bguiz commented 9 years ago

Oh yeah, in your sample ember-cli app, add the vectorSprite options to the EmberApp constructor like so

var app = new EmberApp({
    vectorSprite: {
        debug: true,
        inputDir: 'images', //where the svg images are located
        outputDir: 'assets', //where the output css and images

should be placed spritedir: '.', //image output in the same directory as outputdir sprite: 'sprites', //image file will be named sprites.svg prefix: 'icon', verbose: 1, } });

W: http://bguiz.com

On 23 December 2014 at 17:16, Brendan Graetz brendan.graetz@gmail.com wrote:

OK, another push has gone in, this time to both broccoli-sprite and ember-sprite, in their respective feature/svg branches.

At the moment, it successfully runs the SVG sprite tool, generating the required CSS and SVG files. However, merging it back into the main app's broccoli tree still does not work (I have been trying to debug that in vain for the past half an hour). I'm going to have to leave it as that for the moment.

If you - or anyone else here - would like to have a play and figure out where it is going wrong, go for it!

W: http://bguiz.com

On 23 December 2014 at 15:12, Toran Billups notifications@github.com wrote:

Wow thanks for working on this like a boss ! And if I haven't said it already - it's awesome projects like this that help me iterate so quickly with ember! thank you, thank you, thank you!

— Reply to this email directly or view it on GitHub.

toranb commented 9 years ago

Awesome! I'll give it a look myself after I get a break from the holiday travel this week !

toranb commented 9 years ago

Quick update - still trying to work through the issue you mentioned above (thanks for the patience) !

toranb commented 9 years ago

After playing around with your spike I have a few questions/concerns I wanted to run by you

1) As you stated this line seems to be the part that "hangs" the entire setup

https://github.com/bguiz/ember-sprite/blob/feature/svg/index.js#L43

I'm unsure why exactly it hangs because of my lack of experience with Broccoli Trees/how they should work. But because the line above that seemed to work just fine (before this SVG addon was part of the picture)

https://github.com/bguiz/ember-sprite/blob/feature/svg/index.js#L40

... I'm leaning toward the vector.js (broccoli-sprite) code.

The first thing I noticed as part of vector.js is that you had a promise resolve in the callback

https://github.com/bguiz/broccoli-sprite/blob/feature/svg/vector.js#L60

But ... I don't ever see this callback fire (even in my original spike) unless I force an error of some kind. Yet you do the promise resolve step inside that callback ... so I'm curious if this promise is returned ... but nothing legit ever resolves ... did it work correctly?

I see other projects that use svg-sprite tend to use this async setup instead of the resolve callback

https://github.com/jkphl/grunt-svg-sprite/blob/master/tasks/svg_sprite.js#L43

But above you stated that it did indeed dump out the svg/css code. It's possible I'm just a few hours/days behind reading the code but I wanted to touch base that this is returned and the value (not just the promise) is returned using the "resolved" value/output directory like you had it.

Sorry I still don't have a working patch (I did submit a PR for a simple bug I found hacking around tonight)

Thanks again - looking forward to any feedback you may have

bguiz commented 9 years ago

Hi Toran, I have finally had a chance to take a look at this again after a long break. Thanks for the patch, btw.

I have been running into the same issue as before, unable to resolve it, which was merging the generated css and svg files back into the main tree. The problem appears to be that the output directory being something unexpected.

I know that this is not of much help to you now. I was hoping that you might have an ember-cli project published somewhere where you were attempting to use ember-sprite in your main Brocfile.js. WOuld that still happen to be this project, by any chance?

https://github.com/toranb/example-cli-with-phantomjs/

... or would happen to have a more up to date one that you would like me to test against, given that it has been three weeks since I last looked at this?

W: http://bguiz.com

On 31 December 2014 at 16:19, Toran Billups notifications@github.com wrote:

After playing around with your spike I have a few questions/concerns I wanted to run by you

1) As you stated this line seems to be the part that "hangs" the entire setup

https://github.com/bguiz/ember-sprite/blob/feature/svg/index.js#L43

I'm unsure why exactly it hangs because of my lack of experience with Broccoli Trees/how they should work. But because the line above that seemed to work just fine (before this SVG addon was part of the picture)

https://github.com/bguiz/ember-sprite/blob/feature/svg/index.js#L40

... I'm leaning toward the vector.js (broccoli-sprite) code.

The first thing I noticed as part of vector.js is that you had a promise resolve in the callback

https://github.com/bguiz/broccoli-sprite/blob/feature/svg/vector.js#L60

But ... I don't ever see this callback fire (even in my original spike) unless I force an error of some kind. Yet you do the promise resolve step inside that callback ... so I'm curious if this promise is returned ... but nothing legit ever resolves ... did it work correctly?

I see other projects that use svg-sprite tend to use this async setup instead of the resolve callback

https://github.com/jkphl/grunt-svg-sprite/blob/master/tasks/svg_sprite.js#L43

But above you stated that it did indeed dump out the svg/css code. It's possible I'm just a few hours/days behind reading the code but I wanted to touch base that this is returned and the value (not just the promise) is returned using the "resolved" value/output directory like you had it.

Sorry I still don't have a working patch (I did submit a PR for a simple bug I found hacking around tonight)

Thanks again - looking forward to any feedback you may have

— Reply to this email directly or view it on GitHub.

toranb commented 9 years ago

The current hacked together project you listed above (ember-cli-with-phantomjs) is the most I've done with the raw node module for svg work. I haven't /didn't use Broccoli in that app so I'm not sure (yet) how that works exactly. Admittedly I need to invest some time learning Broccoli and how it works with this node module