jonathantneal / svg4everybody

Use external SVG spritemaps today
https://jonneal.dev/svg4everybody/
Other
3.29k stars 353 forks source link

Enabling polyfill breaks internal use tags #142

Closed BenDTU closed 7 years ago

BenDTU commented 7 years ago

Using svg4 everybody with polyfill: true causes internal use tags to break. I've replicated this at https://jsfiddle.net/bu5u7s57/ using the exact code on the example page, but with polyfill enabled. The code assumes all 'use' tags are external and tries to parse them appropriately:

var srcSplit = src.split("#"), url = srcSplit.shift(), id = srcSplit.join("#");
                        // if the link is external
                        if (url.length) {
                            // get the cached xhr request
                            var xhr = requests[url];
                            // ensure the xhr request exists
                            xhr || (xhr = requests[url] = new XMLHttpRequest(), xhr.open("GET", url), xhr.send(),
                                xhr._embeds = []), // add the svg and id as an item to the xhr embeds list
                                xhr._embeds.push({
                                    parent: parent,
                                    svg: svg,
                                    id: id
                                }), // prepare the xhr ready state change event
                                loadreadystatechange(xhr);
                        } else {
                            // embed the local id into the svg
                            embed(parent, document.getElementById(id));
                        }

Disabling polyfill causes everything to work as expected.

petebarr commented 7 years ago

I've just noticed exactly the same thing which unfortunately limits this plugin. I heavily rely on this plugin with polyfill: true and to fix this would be awesome!

timeiscoffee commented 7 years ago

@BenDTU @petebarr seems like the embed (which is suppose to tag the use's local href and embed it) is not working properly. Let me put in a fix for it and put up a review for you to look at.

timeiscoffee commented 7 years ago

Fixed in https://github.com/jonathantneal/svg4everybody/commit/48197f52f2b53ae30a1e7e1cca3f9d5409de5f91!

BenDTU commented 7 years ago

@timeiscoffee Thanks - still a couple of issues for us which I think are unrelated to this particular fix, I'll stick up a separate issues for them.

petebarr commented 7 years ago

I'm still getting the same issue here unfortunately.

timeiscoffee commented 7 years ago

@petebarr could you clarify the issue that you are seeing? Test case that was linked at the top of this thread is working now: https://jsfiddle.net/bu5u7s57/

petebarr commented 7 years ago

I've added a demo here: http://codepen.io/petebarr/pen/e84411ac73fbda10028d20ee0032ea9c

If you toggle between false and true on the polyfill you will see the inline use xlink:href="#markers" appearing/disappearing.

I'm using v2.1.7.

BenDTU commented 7 years ago

Looks like the issue here is due to #145. The contents of #markers are being appended to the end of the div, but the stroke colour applied within the use tag is lost due to the way svg4everybody embeds.