w0rm / gulp-svgstore

Combine svg files into one with symbol elements
https://www.npmjs.com/package/gulp-svgstore
645 stars 33 forks source link

defs included outside the symbol #85

Closed muhamed87 closed 7 years ago

muhamed87 commented 7 years ago

Hi guys,

I'm facing an issue when using the svgstore that the defs of all icons included in a global defs tag, then the icon symbols included under the defs.

As I don't need to include all icons in all pages because of the performance and due to the huge amount of icons we are using in the project we are working on, I have to include each group of icons in their pages so I get all the icons needed from the symbols and send them to the team that is responsible for including the icons in the pages.

My problem is that I have to search for each icon defs to include it as well before these symbols, is there any way to include the defs of each iconin its icon symbol instead of make it globally???

Waiting your reply ASAP

Thanks in advance

w0rm commented 7 years ago

@muhamed87 the defs must be outside the symbol there's nothing we can do about this.

I believe the solution would be to build multiple bundles.

muhamed87 commented 7 years ago

well that mean I should have to create multiple bundles, what if I should have 50 or more bundles, that mean I should create about 50 gulp task to these bundles so that I can have a solution for my issue?!

Seems that it is not a good solution, as it is created and supported to use it as a gulp task that is already done to make automated tasks, and your suggestion makes me to create lots of gulp tasks and each time I have a new page with new icons then I must create a new bundle and gulp task for it!!

I'm using cleanID, and I think there is no problem to have each defs of each icon included inside its generated symbol as its ID will be different

Is there any other solution please?

w0rm commented 7 years ago

well that mean I should have to create multiple bundles

Isn't it what you need? To have a bundle for each page?

It doesn't necessary have to be multiple tasks, you can create multiple gulp streams in a single task and then merge the streams together.

Using cleanupIDs you can always prefix the id with the bundle name, so they will be unique.

muhamed87 commented 7 years ago

Hey Andrey,

What if I commented the following code in the index.js

if ($defs.length > 0) {
    $combinedDefs.append($defs.contents())
    $defs.remove()
}

Because I think it fixes my issue and each symbol now has its own defs inside it, and I don't know if it will affect any thing other than it don't make the defs globally

w0rm commented 7 years ago

Did you read the thread that I linked to in my first comment? I'm not going to reintroduce regression in the library. This is how it works, it moves defs outside the symbols to fix bugs.