Comments (40)
svgo does something similar https://github.com/svg/svgo/blob/master/plugins/cleanupIDs.js
from gulp-svgstore.
@pastjean can we accomplish similar with cheerio?
from gulp-svgstore.
I guess so, (i've just started playing with cheerio yesterday while fixing my regression on svgstore 👅
something like getting all elements with id's
with id's $('[id]')
all element with xlink:href
with $('[xlink:href])
and elements with style that references url (probably a style parser would be needed here) then rename the ids and all xlinkhref,style accordingly.
from gulp-svgstore.
Yeah, having to parse the styles kinda holds me back from implementing this feature. I also checked svgo code, if only they allowed to specify prefix for the ids. Maybe we shouldn't solve this issue, but rather somehow create a new plugin for svgo and run it as a gulp task before passing to gulp-svgstore.
from gulp-svgstore.
I would rather do that! Sometimes you want the duplicated ids so you can reuse some elements inside of others or you don't want the renaming to occur
Say
file 1 :
<svg>
<symbol id="niceshape" />
<rect/>
</symbol>
<use xlink:href="niceshape"/>
</svg>
file 2 (redefines same symbol):
<svg>
<symbol id="niceshape" />
<rect/>
</symbol>
<use xlink:href="niceshape"/>
</svg>
then you filter with cheerio in gulp to remove duplicate symbols
renaming aggressively would make this impossible (of course could be an option but then... better a separate gulp plugin)
from gulp-svgstore.
What do you think of grunt-svgstore's approach for solving this problem? They append a file hash to each ID in order to make sure its unique within each SVG symbol.
from gulp-svgstore.
@Jakobud this is not an issue, a hash or a filename will make ids unique. The most difficult task is to change all the references in styles and attributes.
from gulp-svgstore.
It wouldn't be hard to do a find/replace in the content. Just find all unique IDs, append hash to each one, and then go through and replace the string everywhere they are used. Would you be interested in a PR that does this? Honestly this would be fairly easy to whip up if you are interested.
from gulp-svgstore.
@Jakobud it's not that simple, these occurrences should be aware of the context, you definitely don't want to replace it inside the text node.
from gulp-svgstore.
Maybe we are talking about different issues here... I'm not sure. What I'm talking about is when you have something like a <linearGradient id="whatever"....
in more than one of your SVGs. After svgstore combines everything you now have multiple linearGradients with the same id and when you try to use them like fill="url(#whatever)"
. And the SVGs end up using the wrong linear gradient since IDs need to be unique. What exactly are you talking about regarding text nodes?
from gulp-svgstore.
#whatever may occur in different places, but not all of them have to be replaced
from gulp-svgstore.
Can you give an example of where #whatever
is used but you wouldn't want to change it? Do you mean like
<text>This is my #whatever text</text>
? If we can come up with a small list of examples it wouldn't be too hard to write this and tests for it as well.
from gulp-svgstore.
I think it is easier to list all the places where it should be changed (or just check what svgo does). But for this we need to parse inline css. I still doubt if it should be a part of svgstore, or a separate svgo plugin similar to https://github.com/svg/svgo/blob/master/plugins/cleanupIDs.js
from gulp-svgstore.
Looks like grunt-svgstore fixes this problem like this:
https://github.com/FWeinb/grunt-svgstore/blob/master/tasks/svgstore.js#L143
It finds url(#...)
and xlink:href=#
and replaces the id's with a unique id.
And here
https://github.com/FWeinb/grunt-svgstore/blob/master/tasks/svgstore.js#L140
is there it changes the id of the element to the unique id.
from gulp-svgstore.
I still doubt if it should be a part of svgstore, or a separate svgo plugin
I have to disagree with this I think. The problem that we are facing is a direct result of concatenating multiple SVGs together via svgstore. Any one out there who uses a vector program like Illustrator to make multiple SVGs that have elements with ids in them will face the same issue once they use svgstore to combine them. Solving the problem by making a plugin for svgo seems like you would be requiring the user to install another dependency just to fix a problem in svgstore.
This is a great gulp package. I really like it a lot and I'm personally and professionally relying on it for web projects. I just want to do everything I can to make it perfect. The grunt version has this issue figured out and I think the same could be applied here. I'm going to work on it this weekend and see what you think.
from gulp-svgstore.
Okay I threw this together tonight
It's actually really simple. For each SVG, before concatenation, it looks for any id's that are referenced using url(#someid)
. It then replaces all instances of someid
with idAttr-someid
. So it basically just appends the filename to the front of the id. So for circle-with-gradient.svg
that has <linearGradient id="SVGID_1_" ...
every instance of SVGID_1_
gets replaced with circle-with-gradient-SVGID_1_
. This ensures that the element with that ID will only be referenced within that symbol after all the SVGs are concatenated.
This approach is also nice because it does not blatantly replace ids that are not referenced elsewhere. For example if you had <circle id=mycircle...>
as long as mycircle
isn't referenced elsewhere in the SVG, then it's left alone. This is important for when you want to select/alter/animate individual SVG elements by it's ID in a webpage. Obviously altering that id would be undesirable.
It seems to work great for everything I have thrown at it so far. I have tried it with gradients, filters, etc. Can you guys think of any other test cases I can throw at this? I'm working on putting together a test for it.
Thoughts? Opinions? I think it's a pretty simple but solid approach.
from gulp-svgstore.
@Jakobud what if there is a <style>
tag that styles element based on its id? Maybe we can ignore this, but then we need to provide a hint to the user. Lets start with tests, but include false positive matches, e.g. same id, but in the places where it shouldn't be replaced. Your <text>This is my #whatever text</text>
is a good example. Also, some attributes where arbitrary text may be put, e.g. xlink:title
.
from gulp-svgstore.
I think the <text>
case won't be difficult to handle. I just need to change my global search/replace to instead just search/replace on element attributes instead. The style tag could be tricky. For example, can you do this?
<linearGradient id="mygradient"...>...</linearGradient>
<style>
circle {
fill: url(#mygradient);
}
</style>
That would be tricky to take care of without introducing a css parser I think.... I'll mess around with this.
from gulp-svgstore.
Okay added an update that takes care of text in <text>
and id's in <style>
tags. Still investigating use cases of xlink:title
and trying to figure out your tests. Some tests on the master branch were failing for me before I made any changes.
from gulp-svgstore.
Okay so I think any type of thing that uses xlink:title
should be covered because it's an attribute just like any other attribute:
id="#someid"
fill="url(#someid)"
style="fill:url(#someid)"
etc
from gulp-svgstore.
Okay I made some changes and added several tests
https://github.com/Jakobud/gulp-svgstore/commits/feature/unique-element-ids
The tests are:
Should rename referenced ids in element attributes
Should rename referenced ids from definition tags
Should rename referenced ids in style tags
Should not rename id in text elements
Should not rename un-referenced ids
It's working very well so far. Do you guys see any other coverage I need to take care of?
from gulp-svgstore.
@Jakobud I am really worried of merging this change, because it may potentially break svg.
- What if the
id
that you want to rename is only a part of the actualid
, e.g. you need to replaceSVGID_1_
withcircle-with-id-SVGID_1_
, but there is alreadySVGID_1_something
, that will be mistakenly replaced withcircle-with-id-SVGID_1_something
. - If the id is
url
, will thenfill="url(#SVGID_1_)"
becomefill="circle-with-id-url(#SVGID_1_)"
? - The intention of only replacing referenced ids contradicts with the task of making all ids unique, i.e. we may get duplicated ids in the result, if they were un-referenced.
I suggest you to implement a separate npm package that exports one function renameIds
that accepts svg as cheerio object, and rename function to be called on each id. Then you can pipe it through gulp-cheerio
like this:
gulp.task('svg', function () {
return gulp
.src('test/src/*.svg')
.pipe(cheerio({
run: function ($, file) {
renameIds($, function (id) {
return path.basename(file.relative, path.extname(file.relative)) + '-' + id;
})
},
parserOptions: { xmlMode: true }
}))
.pipe(svgstore())
.pipe(gulp.dest('test/dest'))
})
As soon as it is stable enough, we can use it inside gulp-svgstore.
from gulp-svgstore.
- That is a use case I need to adjust the code for. Thanks,
- No, it renamed it correctly as you can see in the tests
- This is maybe worth a discussion but I don't think changing un-referenced ids makes sense. Un-referenced id's don't cause any problems that I can see. It's the non-unique referenced id's that svgstore doesn't handle well, which is what I'm trying to fix. Also, when someone is using an svg with un-referenced ids, they may be planning on using those ids to programmatically animate those elements using CSS or something like that (I know I have done this). So changing those might be frustrating to the user cause now they have to go into their source and change them there too. Changing referenced ids seems to be fine though because it's something that the user never needs to know or care about...
One possible exception I can think of is if a user was programmatically changing an element fromfill="url(#SVGID_1_)"
to fill="url(#SVGID_2_)"
or something like that. If svgstore changes the ids then the user would need to know about that ahead of time. I think thats an edge case but it's probably worth a discussion.
I'm a little frustrated here. You seem to be pretty hesitant to want to solve this problem. I mean it's your project and that's fine, but do you understand that svgstore generates broken svgs in the case of non-unique referenced ids? I don't even know if it's valid to the svg spec to have multiple referenced elements with the same id in one svg. Take a look here for a simple example:
http://jsbin.com/wecekodeqo/edit?html,output
The first circle is the wrong color because the second linearGradient
had the same id as the first one and therefore takes priority. The ids are used globally within the svg, not just within each symbol.
When anyone uses something like Adobe Illustrator or Inkscape or whatever to generate SVGs, things like <linearGradient>
s are always (automatically) named stuff like, SVGID_1_
, SVGID_2_
, SVGID_3_
, etc. So when svgstore combined multiple files with these same exact ids, you have problems. The absolutely only solution is to manually go through an SVG in an editor and rename the IDs to something unique every time you generate a new one. This especially sucks when you have to make a change to an SVG in Illustrator and save out a new one. Time to go manually rename all those ids again so that they work...
Like I said, it's your project and you can do whatever you want with it. It's a great gulp plugin. But I'm going to make some more improvements to what I'm working on but I'm pretty much done as it is passing my tests and working perfectly for everything I have been throwing at it so far. I'll just be using my branch for my projects. Thank you for the great Gulp plugin.
from gulp-svgstore.
@Jakobud I'm really sorry for frustrating you. I completely understand the issue that you're trying to solve. My only concern is merging something unstable into the project, and having to maintain this afterwards for other use cases, like point 1. Thats why I suggested a safer path of doing this, by splitting functionality and maintaining it separately.
What about doing it the way I proposed, and I will integrate such module in gulp-svgstore from the day 1, but make it optional?
from gulp-svgstore.
Because svgo already has cleanupIDs
plugin that supports prefixes, this seems the best way of solving the issue without having to reinvent the bicycle:
var gulp = require('gulp');
var SVGO = require('svgo');
var svgstore = require('gulp-svgstore');
var stream = require('stream');
var path = require('path');
function prefixIds () {
var st = new stream.Transform({objectMode: true});
st._transform = function (file, enc, done) {
var data = file.contents.toString();
var prefix = path.basename(file.relative, path.extname(file.relative));
var svgo = new SVGO({
plugins: [{
cleanupIDs: {
prefix: prefix + '-',
minify: true
}
}]
});
svgo.optimize(data, function (result) {
file.contents = new Buffer(result.data);
done(null, file);
});
}
return st;
}
gulp.task('default', function () {
gulp.src('svg/*.svg')
.pipe(prefixIds())
.pipe(svgstore())
.pipe(gulp.dest('.'))
});
Bonus point, this will not only prefix ids, but can also minify them.
Below is the output when using svgs from @Jakobud tests:
<svg xmlns="http://www.w3.org/2000/svg">
<defs>
<linearGradient id="circle-a" y2="100%">
<stop offset="0" stop-color="#FFF"/>
<stop offset="1"/>
</linearGradient>
<linearGradient id="square-a" y2="100%">
<stop offset="0" stop-color="#FFF"/>
<stop offset="1"/>
</linearGradient>
</defs>
<symbol id="circle">
<circle fill="url(#circle-a)" cx="2" cy="2" r="1"/>
</symbol>
<symbol id="square">
<path fill="url(#square-a)" d="M1 1h2v2H1z"/>
</symbol>
</svg>
from gulp-svgstore.
Yeah that looks like a good way of doing this. I didn't realize it had the potential to append to ids. Good call!
from gulp-svgstore.
@Jakobud yeah svgo is really powerful and minifying step is needed anyway in svg pipeline. I wonder if this should be in its own gulp plugin, because it needs to configure svgo on a per-file basis.
Can be done in a pull request to @ben-eb gulp-svgmin, that allows passing a function that receives file and returns svgo options, so something like this would be possible:
gulp.src('svg/*.svg')
.pipe(svgmin(function (file) {
var prefix = path.basename(file.relative, path.extname(file.relative));
return {
plugins: [{
cleanupIDs: {
prefix: prefix + '-',
minify: true
}
}]
}
}))
.pipe(svgstore())
.pipe(gulp.dest('.'))
from gulp-svgstore.
Sure, why not. Send a PR with the necessary improvements. 😄
from gulp-svgstore.
Hmmm is there no way to grab the basename of the current file in the stream to pass it to a gulp plugin? Seems like there would be a way to do something like that...
from gulp-svgstore.
@Jakobud plugin creates a transform stream with predefined svgo instance, it needs a callback per transformation for a custom behavior. I see no other way of doing this
from gulp-svgstore.
@ben-eb I sent PR, will update gulp-svgstore readme when you publish it, and then close this issue.
from gulp-svgstore.
Thanks, sorry I didn't have a chance to put together a PR for that. I was trying to figure out a way to pass a closure as the prefix name and pull in the current source file basename. But I guess maybe you can't do that with gulp.
from gulp-svgstore.
By the way, does that cleanupIDs plugin rename ALL ids? Even non-referenced ones?
from gulp-svgstore.
@Jakobud I think that by default it even removes unused ids. It can be turned off, but then it will minify and prefix all of them. You can try to implement your own svgo plugin, based on https://github.com/svg/svgo/blob/master/plugins/cleanupIDs.js
from gulp-svgstore.
Ah okay. Yeah removing unused ids could be undesirable in the case that you are trying to select certain elements to color or animate them using CSS.
from gulp-svgstore.
That's a bit strange use case, do you inject combined file into html and then style it from page's CSS?
I'd rather not alter symbols' contents with CSS, there are several options, you can remove fills, and they will inherit from svg tag that contains use tag. If you need to color different elements, you may separate them into files that will become symbols, and then put multiple use tags inside your svg tag.
from gulp-svgstore.
Yeah just pull in your SVG sprite into your html via ajax (and optionally store/load it in localstorage).
Also, you don't need to remove fills if you do stuff like use path { fill: inherit; }
. That will allow you to override the fills that are defined in the symbol.
http://tympanus.net/codrops/2015/07/16/styling-svg-use-content-css/
from gulp-svgstore.
@Jakobud restyling all paths isn't flexible either, sometimes I need to change the color of some paths, but keep the rest.
It will also break when in the future all browsers will support linking to symbols from external svg.
from gulp-svgstore.
ids and classes are how I target specific paths and elements.
from gulp-svgstore.
Depends on what do you want to style, if you target contents of svgstored symbols from page's css, then you won't be able to change fill later in different places where you use them.
from gulp-svgstore.
Related Issues (20)
- Combine svg's to sprite - apply class attribute to symbols HOT 1
- Incredible "fill" attribute HOT 1
- Name and link for html5boilerplate visuallyhidden class is obsolete in README
- Options for including title and desc tags
- Rendering issues due to <svg> style not being trasferred HOT 3
- css-what dependency is vulnerable to Denial of Service HOT 1
- Fork or pull request for migrate to stack? HOT 2
- Apply class attribute to symbols HOT 1
- "viewBox → viewbox" is bad HOT 5
- <def><style> stops fill: anycolor HOT 3
- defs included outside the symbol HOT 5
- Defs cleanup?? Defs are missing after running svgstore HOT 1
- gulp4 alpha.3 HOT 3
- Styling individual paths through css HOT 1
- Moving away from the Buffer constructor HOT 2
- Content loaded via ajax shows empty icons HOT 3
- UnhandledPromiseRejectionWarning when more than one SVG in the source directory HOT 1
- Problem with globs HOT 2
- Update README to propose automatic fix for `clipPath` issues HOT 1
- No preview files available HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gulp-svgstore.