Skip to content

Commit

Permalink
Disable view flattening when the view has event handlers on Android
Browse files Browse the repository at this point in the history
Summary:
The views with touch event props are currently flattened by Fabric core, as we don't take event listeners into account when calculating whether the view should be flattened. This results in a confusing situation when components with touch event listeners (e.g. `<View onTouchStart={() => {}} /> `) or ones using `PanResponder` are either ignored (iOS) or cause a crash (Android).

This change passes touch event props to C++ layer and uses them to calculate whether the view node should be flattened or not. It also refactors events to be kept as a singular bitset with 32 bit (~`uint32_t`).

Changelog: [Changed][General] Avoid flattening nodes with event props

Reviewed By: sammy-SC

Differential Revision: D34005536

fbshipit-source-id: 96255b389a7bfff4aa208a96fd0c173d9edf1512
  • Loading branch information
Andrei Shikov authored and facebook-github-bot committed Feb 10, 2022
1 parent 9ed2df6 commit 980c52d
Show file tree
Hide file tree
Showing 10 changed files with 352 additions and 43 deletions.
55 changes: 49 additions & 6 deletions Libraries/NativeComponent/PlatformBaseViewConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ const PlatformBaseViewConfig: PartialViewConfigWithoutName =
registrationName: 'onAccessibilityAction',
},
topPointerEnter: {
registrationName: 'pointerenter',
registrationName: 'onPointerEnter',
},
topPointerLeave: {
registrationName: 'pointerleave',
registrationName: 'onPointerLeave',
},
topPointerMove: {
registrationName: 'pointermove',
registrationName: 'onPointerMove',
},
onGestureHandlerEvent: DynamicallyInjectedByGestureHandler({
registrationName: 'onGestureHandlerEvent',
Expand Down Expand Up @@ -219,9 +219,31 @@ const PlatformBaseViewConfig: PartialViewConfigWithoutName =
position: true,
onLayout: true,

pointerenter: true,
pointerleave: true,
pointermove: true,
// Pointer events
onPointerEnter: true,
onPointerLeave: true,
onPointerMove: true,

// PanResponder handlers
onMoveShouldSetResponder: true,
onMoveShouldSetResponderCapture: true,
onStartShouldSetResponder: true,
onStartShouldSetResponderCapture: true,
onResponderGrant: true,
onResponderReject: true,
onResponderStart: true,
onResponderEnd: true,
onResponderRelease: true,
onResponderMove: true,
onResponderTerminate: true,
onResponderTerminationRequest: true,
onShouldBlockNativeResponder: true,

// Touch events
onTouchStart: true,
onTouchMove: true,
onTouchEnd: true,
onTouchCancel: true,

style: ReactNativeStyleAttributes,
},
Expand Down Expand Up @@ -456,6 +478,27 @@ const PlatformBaseViewConfig: PartialViewConfigWithoutName =
onAccessibilityAction: true,
onAccessibilityEscape: true,
onAccessibilityTap: true,

// PanResponder handlers
onMoveShouldSetResponder: true,
onMoveShouldSetResponderCapture: true,
onStartShouldSetResponder: true,
onStartShouldSetResponderCapture: true,
onResponderGrant: true,
onResponderReject: true,
onResponderStart: true,
onResponderEnd: true,
onResponderRelease: true,
onResponderMove: true,
onResponderTerminate: true,
onResponderTerminationRequest: true,
onShouldBlockNativeResponder: true,

// Touch events
onTouchStart: true,
onTouchMove: true,
onTouchEnd: true,
onTouchCancel: true,
}),
},
};
Expand Down
25 changes: 25 additions & 0 deletions React/Views/RCTViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,29 @@ - (RCTShadowView *)shadowView

RCT_EXPORT_SHADOW_PROPERTY(direction, YGDirection)

// The events below define the properties that are not used by native directly, but required in the view config for new
// renderer to function.
// They can be deleted after Static View Configs are rolled out.

// PanResponder handlers
RCT_CUSTOM_VIEW_PROPERTY(onMoveShouldSetResponder, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onMoveShouldSetResponderCapture, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onStartShouldSetResponder, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onStartShouldSetResponderCapture, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderGrant, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderReject, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderStart, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderEnd, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderRelease, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderMove, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderTerminate, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onResponderTerminationRequest, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onShouldBlockNativeResponder, BOOL, RCTView) {}

// Touch events
RCT_CUSTOM_VIEW_PROPERTY(onTouchStart, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onTouchMove, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onTouchEnd, BOOL, RCTView) {}
RCT_CUSTOM_VIEW_PROPERTY(onTouchCancel, BOOL, RCTView) {}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -486,18 +486,103 @@ private void logUnsupportedPropertyWarning(String propName) {
FLog.w(ReactConstants.TAG, "%s doesn't support property '%s'", getName(), propName);
}

@ReactProp(name = "pointerenter")
public void setPointerEnter(@NonNull T view, @Nullable boolean value) {
@ReactProp(name = "onPointerEnter")
public void setPointerEnter(@NonNull T view, boolean value) {
view.setTag(R.id.pointer_enter, value);
}

@ReactProp(name = "pointerleave")
public void setPointerLeave(@NonNull T view, @Nullable boolean value) {
@ReactProp(name = "onPointerLeave")
public void setPointerLeave(@NonNull T view, boolean value) {
view.setTag(R.id.pointer_leave, value);
}

@ReactProp(name = "pointermove")
public void setPointerMove(@NonNull T view, @Nullable boolean value) {
@ReactProp(name = "onPointerMove")
public void setPointerMove(@NonNull T view, boolean value) {
view.setTag(R.id.pointer_move, value);
}

@ReactProp(name = "onMoveShouldSetResponder")
public void setMoveShouldSetResponder(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onMoveShouldSetResponderCapture")
public void setMoveShouldSetResponderCapture(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onStartShouldSetResponder")
public void setStartShouldSetResponder(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onStartShouldSetResponderCapture")
public void setStartShouldSetResponderCapture(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderGrant")
public void setResponderGrant(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderReject")
public void setResponderReject(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderStart")
public void setResponderStart(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderEnd")
public void setResponderEnd(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderRelease")
public void setResponderRelease(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderMove")
public void setResponderMove(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderTerminate")
public void setResponderTerminate(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onResponderTerminationRequest")
public void setResponderTerminationRequest(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onShouldBlockNativeResponder")
public void setShouldBlockNativeResponder(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onTouchStart")
public void setTouchStart(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onTouchMove")
public void setTouchMove(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onTouchEnd")
public void setTouchEnd(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}

@ReactProp(name = "onTouchCancel")
public void setTouchCancel(@NonNull T view, boolean value) {
// no-op, handled by JSResponder
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -808,19 +808,19 @@ public void setShouldNotifyOnLayout(boolean shouldNotifyOnLayout) {
super.setShouldNotifyOnLayout(shouldNotifyOnLayout);
}

@ReactProp(name = "pointerenter")
@ReactProp(name = "onPointerEnter")
public void setShouldNotifyPointerEnter(boolean value) {
// This method exists to inject Native View configs in RN Android VR
// DO NOTHING
}

@ReactProp(name = "pointerleave")
@ReactProp(name = "onPointerLeave")
public void setShouldNotifyPointerLeave(boolean value) {
// This method exists to inject Native View configs in RN Android VR
// DO NOTHING
}

@ReactProp(name = "pointermove")
@ReactProp(name = "onPointerMove")
public void setShouldNotifyPointerMove(boolean value) {
// This method exists to inject Native View configs in RN Android VR
// DO NOTHING
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@
return MapBuilder.builder()
.put("topContentSizeChange", MapBuilder.of(rn, "onContentSizeChange"))
.put("topLayout", MapBuilder.of(rn, "onLayout"))
.put("topPointerEnter", MapBuilder.of(rn, "pointerenter"))
.put("topPointerLeave", MapBuilder.of(rn, "pointerleave"))
.put("topPointerMove", MapBuilder.of(rn, "pointermove"))
.put("topPointerEnter", MapBuilder.of(rn, "onPointerEnter"))
.put("topPointerLeave", MapBuilder.of(rn, "onPointerLeave"))
.put("topPointerMove", MapBuilder.of(rn, "onPointerMove"))
.put("topLoadingError", MapBuilder.of(rn, "onLoadingError"))
.put("topLoadingFinish", MapBuilder.of(rn, "onLoadingFinish"))
.put("topLoadingStart", MapBuilder.of(rn, "onLoadingStart"))
Expand Down
19 changes: 1 addition & 18 deletions ReactCommon/react/renderer/components/view/ViewProps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,7 @@ ViewProps::ViewProps(
"onLayout",
sourceProps.onLayout,
{})),
pointerEnter(convertRawProp(
context,
rawProps,
"pointerenter",
sourceProps.pointerEnter,
{})),
pointerLeave(convertRawProp(
context,
rawProps,
"pointerleave",
sourceProps.pointerLeave,
{})),
pointerMove(convertRawProp(
context,
rawProps,
"pointermove",
sourceProps.pointerMove,
{})),
events(convertRawProp(context, rawProps, sourceProps.events, {})),
collapsable(convertRawProp(
context,
rawProps,
Expand Down
6 changes: 1 addition & 5 deletions ReactCommon/react/renderer/components/view/ViewProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,7 @@ class ViewProps : public YogaStylableProps, public AccessibilityProps {
EdgeInsets hitSlop{};
bool onLayout{};

bool pointerEnter{};

bool pointerLeave{};

bool pointerMove{};
ViewEvents events{};

bool collapsable{true};

Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/react/renderer/components/view/ViewShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ void ViewShadowNode::initialize() noexcept {

bool formsView = formsStackingContext ||
isColorMeaningful(viewProps.backgroundColor) ||
isColorMeaningful(viewProps.foregroundColor) || viewProps.pointerEnter ||
viewProps.pointerLeave || viewProps.pointerMove ||
isColorMeaningful(viewProps.foregroundColor) ||
viewProps.events.bits.any() ||
!(viewProps.yogaStyle.border() == YGStyle::Edges{}) ||
!viewProps.testId.empty();

Expand Down
41 changes: 41 additions & 0 deletions ReactCommon/react/renderer/components/view/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,54 @@
#include <react/renderer/graphics/Color.h>
#include <react/renderer/graphics/Geometry.h>
#include <array>
#include <bitset>
#include <cmath>

namespace facebook {
namespace react {

enum class PointerEventsMode { Auto, None, BoxNone, BoxOnly };

struct ViewEvents {
std::bitset<32> bits{};

enum class Offset : std::size_t {
// Pointer events
PointerEnter = 0,
PointerMove = 1,
PointerLeave = 2,

// PanResponder callbacks
MoveShouldSetResponder = 3,
MoveShouldSetResponderCapture = 4,
StartShouldSetResponder = 5,
StartShouldSetResponderCapture = 6,
ResponderGrant = 7,
ResponderReject = 8,
ResponderStart = 9,
ResponderEnd = 10,
ResponderRelease = 11,
ResponderMove = 12,
ResponderTerminate = 13,
ResponderTerminationRequest = 14,
ShouldBlockNativeResponder = 15,

// Touch events
TouchStart = 16,
TouchMove = 17,
TouchEnd = 18,
TouchCancel = 19,
};

constexpr bool operator[](const Offset offset) const {
return bits[static_cast<std::size_t>(offset)];
}

std::bitset<32>::reference operator[](const Offset offset) {
return bits[static_cast<std::size_t>(offset)];
}
};

enum class BackfaceVisibility { Auto, Visible, Hidden };

enum class BorderStyle { Solid, Dotted, Dashed };
Expand Down
Loading

0 comments on commit 980c52d

Please sign in to comment.