Click Support about react-svg HOT 7 CLOSED

Hey @ChrisGarber-Medlever, glad to hear it's working well for you.

One way you can do what you're after right now is to render ReactSVG inside a wrapping element, and handle the click event there. There's a runnable example in this branch if you want to check it out. Relevant commit is here.

Downside is you will be rendering that extra element, not sure if that's going to be a problem for you in your particular use case though.

Just as an aside re why I'm not making the API change immediately: I'm slightly hesitant to expand the API unless it seems really worthwhile (I'm still on the fence about that wrapperClassName addition actually 😆 ). So I'm definitely open to it, but just need to mull it over some. In the meantime, hope that suggestion above helps somewhat...

from react-svg.

A wrapping element is the work around I've been using. Not a big deal but certainly less intuitive. Here's my argument for this feature; this library acts as my SVG tags. I like to think we're moving towards a world where svgs are considered elements just like divs, etc. It's very natural for me to hook up a click handler on a div, so why not in an SVG element? I really like the wrapperClassName feature and use it a lot because our application relied on parent element classes. If you have a wrapper div already, you might as well make full use of it. Also it's only a passthrough, should only be like 3 lines of changes and a little documentation.

That said having a wrapping element is fairly low impact. It'll just save me code in the long run as I won't have to have a wrapper for the wrapper div.

from react-svg.

Thanks for taking the time to explain things more from your POV @ChrisGarber-Medlever.

As you may have guessed, from my end this is more of a philosophical argument as opposed to an ease of implementation one. I could probably have helped things by providing a bit more in the way of usage docs too.

There's something about a potentially ever-expanding component API that doesn't sit right with me (e.g. adding documented API props for class names, when that's redundant for people using inline styles). It feels like it's better to push things like this back into user-land where specific behaviour can be composed somehow, rather than try to accomodate every use case within this component. Trouble is, at the moment I don't think I can explain it eloquently enough to warrant more than a brief discussion 😅.

Anyways, I think it's fair to expect these sort of feature requests, since it's a reasonably common pattern that you can see implemented in popular UI component libraries for example. So in the interests of moving forward, I quite like the approach taken in the Material UI lib, specifically where they say Other properties (not documented) are applied to the root element(via spread syntax). That feels like a good middle ground, so I'll have a play with that on a new branch... will keep you posted 📫

from react-svg.

@ChrisGarber-Medlever just released v3, now you should be able to add an onClick handler onto the wrapper element. Will leave the issue open for a bit in case you have any more feedback though. Also note I scrubbed that issue-58 branch since it was no longer relevant.

from react-svg.

I love the idea of spreading the props to the wrapper, very clean. Any chance you could release this on npm? I'm gonna submit a pull request to types/react-svg to add an intersection type for us typescript users

from react-svg.

Nice one 👌

v3.0.1 should already be available on npm from that earlier release, I see this via npm info:

'3.0.1': '2018-04-29T03:46:46.147Z'

from react-svg.

@ChrisGarber-Medlever I'll close this issue down now as it looks like we've sorted the things you were after? Happy to reopen though if something's been left out 👌

from react-svg.