Skip to content

Support for attributes on <use> and importNode usage #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 21, 2020

Conversation

timeiscoffee
Copy link
Collaborator

@Eschon
Copy link

Eschon commented Mar 24, 2017

I just tried this out and found a few small problems with it.

First I realized that it doesn't solve #145 completely. @BenDTU mentioned the x and y attributes. Those are copied to the g tag but it doesn't support them. This was also a problem in #122 which I didn't notice.
Fixing this should be possible by just checking for x and y attributes and putting them into a transform="translate(...)"

I also found another interesting Problem. If I have multiple use tags linking the same icon they all get the attributes from the last use tag linking it.

Aside from the mentioned problems it works for me.

@timeiscoffee
Copy link
Collaborator Author

@Eschon thanks for giving it a go.

Re: x, y, I think that should be tackled on as a separate PR.

Re: multiple <use> tag, could you clarify on this? I just tried with copy pasting a <use> tag with same href and changing the attribute, and they seem to preserve their own attributes after fill.

@Eschon
Copy link

Eschon commented May 5, 2017

Sorry it took me such a long time to get back to this.
I created a small test case that shows the problem. You can find it here:
https://github.com/Intermaps/svg4everybody-test

@andi1984
Copy link

What is about this PR? I would love to see it in!

@jonathantneal jonathantneal merged commit b8cb7c1 into jonathantneal:master Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants