react-native-svg icon indicating copy to clipboard operation
react-native-svg copied to clipboard

fix(svg-view): the hitSlop value should use float in android like in ios

Open GrinZero opened this issue 1 year ago • 3 comments

Summary

In IOS, the hitSlop attribute of Svg supports the direct use of hitSlope={4}, but it can cause crashes on Android.The native view is also compatible with this writing style, so I think we should add compatibility in this area, otherwise it may lead to hidden cross end compatibility issues.

Test Plan

  • this will cause crash, and we need to prevent crash
<Svg hitSlop={4} />

What's required for testing (prerequisites)?

Any Android device with sample programs.

What are the steps to reproduce (after prerequisites)?

Check if there are no behavior changes between previous implementation and current

Compatibility

OS Implemented
iOS
Android

Checklist

  • [x] I have tested this on a device and a simulator
  • [ ] I added documentation in README.md
  • [x] I updated the typed files (typescript)
  • [ ] I added a test for the API in the __tests__ folder

GrinZero avatar Jul 03 '24 03:07 GrinZero

Hello @GrinZero, Can we change that like here? I guess it would be a good approach to follow a type like the View component in React native. Also, it doesn't work on the IOS platform, could you implement the same functionality to the IOS platform or do you need help with it?

Thank you.

bohdanprog avatar Jul 03 '24 13:07 bohdanprog

Hello @GrinZero, Can we change that like here? I guess it would be a good approach to follow a type like the View component in React native. Also, it doesn't work on the IOS platform, could you implement the same functionality to the IOS platform or do you need help with it?

Thank you.

Hello 👋 I have checked and found that the actual type support for React Native is different from the documentation. The actual TS type indicates that it supports abbreviations for numbers. Then regarding the issue of "not working on the IOS platform", I will take a look now because when I tried to use the number type, my IOS emulator did not crash.

image

GrinZero avatar Jul 04 '24 15:07 GrinZero

Also, it doesn't work on the IOS platform, could you implement the same functionality to the IOS platform or do you need help with it?

Although it did not cause a crash, it can be confirmed that this attribute is indeed invalid on the IOS platform - silence has expired, whether it is in object or numeric format.

If we want to resolve this issue in this PR, I think I do need help because I am not a professional IOS developer. Actually, I have never been exposed to IOS development before

GrinZero avatar Jul 04 '24 15:07 GrinZero

Hey @GrinZero, Unfortunately, this cannot be merged since the signature of ViewProps is not compatible.

src/fabric/AndroidSvgViewNativeComponent.ts:27:11 - error TS2430: Interface 'NativeProps' incorrectly extends interface 'ViewProps'.
  Types of property 'hitSlop' are incompatible.
    Type 'HitSlop | undefined' is not assignable to type 'Insets | undefined'.
      Type 'number' has no properties in common with type 'Insets'.

However, I've made a PR with a complex solution to this issue https://github.com/software-mansion/react-native-svg/pull/2407 with full iOS and Android support. I would like to highlight your contribution and say thank you! 🚀

jakex7 avatar Aug 19 '24 07:08 jakex7