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

Path onPress prop not handled #34

Open
carlost opened this issue Nov 8, 2018 · 3 comments
Open

Path onPress prop not handled #34

carlost opened this issue Nov 8, 2018 · 3 comments

Comments

@carlost
Copy link

carlost commented Nov 8, 2018

I ran into this when using:

  • react v16.3.1
  • react-native-svg-charts v5.2.0
  • react-native-svg v6.3.1
  • react-native-web v0.8.8
  • svgs v3.2.1

Luckily this isn't a result of some arcane interplay between these packages.

react-native-svg supports a series of touchable props on the Path component. You can see those props in the typescript lib def in their repo. Unfortunately, onPress is not a recognized event handler on the path dom element so react will strip out the property and log an error to the console:

Warning: Unknown event handler property 'onPress'. It will be ignored.

I suspect that once you dig into this issue this will quickly open up a can or worms and require supporting a wider range of TouchableProps on a wider range of SVG dom elements.

let me know if you would like some help putting a PR together. react-native-web has a mixin that adds support for onPress event handlers do dom elements ... the work is pretty extensive. unfortunately, it isn't yet packaged up as an independent module.

@3rd-Eden
Copy link
Contributor

3rd-Eden commented Nov 9, 2018

Yeah, I can see this being problematic. I just don't know if this should be solved in this library or that universal library should be created that can solve the compatibility issue in a more pragmatic way.

While I recognize that is a problem in terms of interop, the amount of code to get compatibility in this area is quite insane with no good options for code sharing. Not really sure what to do here, the last thing I want to is bloat this library just to get event support. Ideally, some sort of interop library should be used/created to tackle this specific issue. E.g.

import { onPress } from 'what-ever-compatilblity-library';
import { Path } from 'svgs';

<Path { ...onPress(() => {}) } />

@carlost
Copy link
Author

carlost commented Nov 9, 2018

@3rd-Eden ... I wholeheartedly agree that creating touch support from scratch is a pretty large undertaking. however, lets imagine a paradise where there was a module that provided generic "onPress" compatibility for the web. if that module existed, wouldn't the ideal place to integrate that functionality be in svgs? that will help increase API compatibility between svgs and react-native-svg and reduce the need for downstream apps to create their own adapters.

@carlost
Copy link
Author

carlost commented Nov 9, 2018

@3rd-Eden ... and to give our use case a little more clarity. we have a react-native app and use react-native-web to provider a browser based UI. we make use of 3rd party libs, such as react-native-charts, that use react-native-svg touch event handlers. we are using svgs to transparently replace the impl of all of the react-native-svg components via webpack aliases. works great.

unfortunately, there are a few gaps in API compatibility and those gaps are being exposed by 3rd party libs, like react-native-charts, that we don't directly control.

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

No branches or pull requests

2 participants