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

feat: make pointer events work on both platforms #1879

Merged
merged 8 commits into from
Sep 30, 2022

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Sep 22, 2022

PR adding proper handling of pointerEvents in the library. Based on the comment by @RSNara :

For all Android components, static ViewConfigs are generated from the components types + BaseVIewConfig.android.js. The BaseViewConfig for all android components is https://github.com/facebook/react-native/blob/c5217f199dfc9db0908a5e4439b7c6b45e17a850/Libraries/NativeComponent/BaseViewConfig.android.js#L288-L297, which doesn't contain the pointerEvents validAttribute. So, React Native SVG components must re-declare the pointerEvents validAttribute. Otherwise, the pointerEvents validAttribute won't be in the static ViewConfig, which means React won't pass that prop to native with Static ViewConfigs.

We needed a way to trick codegen since we added our own pointerEvents with string type, and such prop is already present in ViewProps from react-native, but as a union of strings, so their types did not match. As codegen parses string literals, we include changed ViewProps type, without pointerEvents prop, so we satisfy both codegen and TS.

As for how pointerEvents are handled now:

  • iOS:

Paper: prop is passed in RCTViewManager but it looks like it does not matter what you pass there, since even if you set none, resulting in view.userInteractionEnabled = NO; (https://github.com/facebook/react-native/blob/065db683a20b273ea3656dead29845327637669a/React/Views/RCTViewManager.m#L256), custom implementation of hitTest will not take it into account anyhow.

Fabric: we don’t use pointerEvents prop but it is not a problem since it is not used in the implementation of neither SvgView and RCTViewComponentView (it is used in hitTest method there, but we override it)

  • Android:

Paper: the prop was passed through ReactViewManager and was set on the view directly. But the setter for pointerEvents is not visible outside of ReactViewGroup . After adding Fabric integration for both platforms: BaseViewManager does not implement setter for pointerEvents so we have to add the prop by ourselves. Still, we cannot call the setter from ReactViewGroup since it is not visible, so all the logic for pointerEvents should be migrated to SvgView unfortunately or we should use reflection to get that method.

Despite that, it seems that behavior differs on Android and iOS since setting pointerEvents to none on Android makes the view not clickable, whereas on iOS it does not change anything.

@WoLewicki WoLewicki requested a review from tomekzaw September 30, 2022 11:20
@WoLewicki WoLewicki merged commit 1beacf1 into main Sep 30, 2022
@WoLewicki WoLewicki deleted the @wolewicki/add-pointer-events-to-spec branch September 30, 2022 12:24
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.

2 participants