flobacher / SVGInjector2

Fast, caching, dynamic inline SVG DOM injection library
MIT License
57 stars 8 forks source link

Issues when used with angular #11

Closed Viveur closed 7 years ago

Viveur commented 7 years ago

If angular is present it appears that SVGinjector insists on it being loaded as a module.

Line: 1052 if (typeof angular === 'object') {

If loaded this way the included directive will throw an error anytime it finds a SVG.

TypeError: Cannot read property 'split' of null

Unsure how you feel it'd be best to fix this, but also thinking it might make sense to change the angular integration to be purely optional at the same time, rather than a requirement, so that people can use as a straight JS library if desired.

flobacher commented 7 years ago

should be fixed via #50eaafb please test with the new release v2.0.32

flobacher commented 7 years ago

only a problem when used in combination with angular was fixed, but when angular is present, the angular directive is used to inject every svg on the page that has a src, data-src or the special classname

Viveur commented 7 years ago

Thanks for taking a look at this, looks like that error is fixed, thanks; however, the other part of my comment was regarding the way that SVGInjector is loaded if Angular is present.

A new user to SVGInjector would read the README and assume that in order to get it working they would add:

var mySVGsToInject = document.querySelectorAll('img.inject-me');
new SVGInjector().inject(mySVGsToInject);

Due to if (typeof angular === 'object') { this does not work and throws a ReferenceError: SVGInjector is not defined since it never reaches the code:

else if (typeof window === 'object') { window.SVGInjector = SVGInjector; }

Critically, the only reference to Angular in the docs is its use as a directive, while I (and likely other users) currently still use the format:

<img class="inject-me" src="some.svg">

The directive is of no use in this case since it's scope is too narrow (it only works with <svg>'s)

I can get SVGInjector working but in order to do so I have to both inject it as a module and inject the svgInjectorFactory service under .run, as well as specifying:

var mySVGsToInject = document.querySelectorAll('img.inject-me');
svgInjectorFactory.inject(mySVGsToInject);

While not particularly difficult none of this is documented and my suggestion was in regard to making life simpler for new users. Here's a few suggestions to improve things - any one (or all of them) would probably do the trick :)

Just some ideas... unsure what option you think would be best?

Viveur commented 7 years ago

Actually not 100% sure whether that commit fixed the error I was alluding to. Am still getting:

TypeError: Cannot read property 'split' of null

The changes you made were just above the line that is causing the error:

var imgUrlSplitByFId = imgUrl.split("#");