Skip to content
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

SvgXml implementation for web #1431

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

MatheusrdSantos
Copy link

Summary

Explain the motivation for making this change: here are some points to help you:

  • What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged
    'SvgXml' for web #1279
    Attempted import error: 'SvgXml' is not exported from 'react-native-svg'. #1236

  • What is the feature? (if applicable)
    This PR contains the implementation of SvgXml for web. This web implementation has exactly the same usage as the native implementation.

  • How did you implement the solution?
    The web implementation relies on unstable_createElement and View from react-native-web. So, basically I used unstable_createElement to create an <svg> element with the props that comes from the XML string and set the innerHTML from the <svg> to be the XML content.
    In order to accept react-native props (the same as SvgXml native implementation), I used the <View> to wrap the <svg> and simulate the same behavior.

  • What areas of the library does it impact?
    None. It just brings a new supported component for web.

Test Plan

You can check a working demo here: https://codesandbox.io/s/keen-platform-z40rg

Compatibility

OS Implemented
iOS
Android
Web

Checklist

  • I have tested this on browser

@kofsiwon
Copy link

I hope release this quickly. I need SvgXml for react native web!!

@Sharcoux
Copy link

For those still needing this, you can try our package: @cantoo/rn-svg

@MattFoley
Copy link

@WoLewicki @MatheusrdSantos @Sharcoux Should we merge this in? It seems risky long term to have a fork maintain this functionality. Thanks!

@Sharcoux
Copy link

@MattFoley I'm glad that the project revived.

We can rebase our fork and make it ready for merge, that would be awesome. However, there is a concern you should know about:

We had to wrap the svg inside a View. This has impacts when trying to style the SVG, especially with everything related to layout. We made a system that distribute the styles between the wrapper and the svg itself, but it will very likely introduce problems in some layouts. unstable_createElement would not let us pass down every react-native props to the underlying element. Another valid approach could be to only export the element created with unstable_createElement and give up on unsupported props.

You'll find all the details here: necolas/react-native-web#1686

@MattFoley
Copy link

Hmm, I'll be honest I don't fully understand the debate on RNW, and would need more time to dive into the specifics. I also am using v14.1 of rn-svg, and don't think I can downgrade to 13. I started to rebase your fork up to v14.1 myself, but haven't finished yet, working through some BoB type errors.

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.

None yet

4 participants