search

jonathantneal / svg4everybody

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

Allow arbitrarily nested <use> elements #117

Closed shawnbot closed 7 years ago

shawnbot commented 8 years ago

This PR addresses the use case (heh) for <use> elements that aren't direct descendants of their SVG element, e.g.

<svg>
  <a xlink:href="foo.html">
    <use xlink:href="bar.svg#symbol"/>
  </a>
</svg>

The changes allow for this by removing checks for direct "ancestry" (/svg/i.test(svg.nodeName)) and perform replacements on the <use> element's parent, which also makes it easier to style those elements with CSS descendent selectors.

We've added a test to test/index.html that works, and the code passes linting muster. We've ensured that the validate() and fallback() callbacks still get the SVG element as their second argument, so those should still work as expected—though some tests for those would be very welcome.

shawnbot commented 8 years ago

Hey @jonathantneal, is there anything I can do to move this along? I've also offered to help in #101 if you want to make me a contributor.

shawnbot commented 8 years ago

Anyone want to review this one for me? @jonathantneal?

shawnbot commented 8 years ago

I'd like to use element-closest instead of the inline getSVGAncestor() function here, but we're not set up for browserification of CJS modules yet. That's something to do re: #60 and #114 in a future update.

shawnbot commented 7 years ago

Thanks for the review, @timeiscoffee! I gave you access to the branch just in case you have time to get to this before I do.

timeiscoffee commented 7 years ago

@shawnbot which branch did you give me the access to? I don't see it unfortunately :(

shawnbot commented 7 years ago

@timeiscoffee you should have write access to this fork's use-parent branch. I think that if you can click on the pencil icon on this page then all's good?

timeiscoffee commented 7 years ago

@shawnbot I tried to push to that use-parent branch, but got not authorized error. Trying to edit that README gives me:

You’re editing a file in a project you don’t have write access to. Submitting a change to this file will write it to a new branch in your fork timeiscoffee/svg4everybody, so you can send a pull request..

shawnbot commented 7 years ago

Derf, maybe that feature only works for user forks and not org forks. Sorry! I'm happy to close this PR if you have fixes ready.

timeiscoffee commented 7 years ago

Sure, I would be happy to :)

timeiscoffee commented 7 years ago

Closing as per above discussion, since https://github.com/jonathantneal/svg4everybody/pull/132 has been merged