Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,6 @@ open class GestureHandler {
hitSlop!![HIT_SLOP_BOTTOM_IDX] = bottomPad
hitSlop!![HIT_SLOP_WIDTH_IDX] = width
hitSlop!![HIT_SLOP_HEIGHT_IDX] = height
require(!(hitSlopSet(width) && hitSlopSet(leftPad) && hitSlopSet(rightPad))) {
"Cannot have all of left, right and width defined"
}
require(!(hitSlopSet(width) && !hitSlopSet(leftPad) && !hitSlopSet(rightPad))) {
"When width is set one of left or right pads need to be defined"
}
require(!(hitSlopSet(height) && hitSlopSet(bottomPad) && hitSlopSet(topPad))) {
"Cannot have all of top, bottom and height defined"
}
require(!(hitSlopSet(height) && !hitSlopSet(bottomPad) && !hitSlopSet(topPad))) {
"When height is set one of top or bottom pads need to be defined"
}
}

fun setHitSlop(padding: Float?) {
Expand Down Expand Up @@ -968,68 +956,44 @@ open class GestureHandler {
private const val KEY_MANUAL_ACTIVATION = "manualActivation"
private const val KEY_MOUSE_BUTTON = "mouseButton"
private const val KEY_HIT_SLOP = "hitSlop"
private const val KEY_HIT_SLOP_LEFT = "left"
private const val KEY_HIT_SLOP_TOP = "top"
private const val KEY_HIT_SLOP_RIGHT = "right"
private const val KEY_HIT_SLOP_BOTTOM = "bottom"
private const val KEY_HIT_SLOP_VERTICAL = "vertical"
private const val KEY_HIT_SLOP_HORIZONTAL = "horizontal"
private const val KEY_HIT_SLOP_WIDTH = "width"
private const val KEY_HIT_SLOP_HEIGHT = "height"
private const val KEY_TEST_ID = "testID"
private const val KEY_CANCELS_JS_RESPONDER = "cancelsJSResponder"

/**
* `hitSlop` arrives already normalized by the JS side as
* `[left, top, right, bottom, width, height]`, where `null` marks an edge that was not
* specified. Validation of the `width`/`height` combinations happens in JS as well, so all
* that is left here is converting the values from DIP to pixels.
*/
private fun handleHitSlopProperty(handler: GestureHandler, config: ReadableMap) {
if (config.isNull(KEY_HIT_SLOP)) {
handler.setHitSlop(null)
} else if (config.getType(KEY_HIT_SLOP) == ReadableType.Number) {
val hitSlop = PixelUtil.toPixelFromDIP(config.getDouble(KEY_HIT_SLOP))
handler.setHitSlop(
hitSlop,
hitSlop,
hitSlop,
hitSlop,
GestureHandler.HIT_SLOP_NONE,
GestureHandler.HIT_SLOP_NONE,
)
return
}

// A uniform hit slop stays a plain number on the wire, which spares the bridge the
// wrapper it would allocate for an array.
if (config.getType(KEY_HIT_SLOP) == ReadableType.Number) {
handler.setHitSlop(PixelUtil.toPixelFromDIP(config.getDouble(KEY_HIT_SLOP)))
return
}

val hitSlop = config.getArray(KEY_HIT_SLOP)!!

fun edge(index: Int) = if (hitSlop.isNull(index)) {
GestureHandler.HIT_SLOP_NONE
} else {
Comment thread
m-bert marked this conversation as resolved.
val hitSlop = config.getMap(KEY_HIT_SLOP)!!
var left = GestureHandler.HIT_SLOP_NONE
var top = GestureHandler.HIT_SLOP_NONE
var right = GestureHandler.HIT_SLOP_NONE
var bottom = GestureHandler.HIT_SLOP_NONE
var width = GestureHandler.HIT_SLOP_NONE
var height = GestureHandler.HIT_SLOP_NONE
if (hitSlop.hasKey(KEY_HIT_SLOP_HORIZONTAL)) {
val horizontalPad = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_HORIZONTAL))
right = horizontalPad
left = right
}
if (hitSlop.hasKey(KEY_HIT_SLOP_VERTICAL)) {
val verticalPad = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_VERTICAL))
bottom = verticalPad
top = bottom
}
if (hitSlop.hasKey(KEY_HIT_SLOP_LEFT)) {
left = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_LEFT))
}
if (hitSlop.hasKey(KEY_HIT_SLOP_TOP)) {
top = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_TOP))
}
if (hitSlop.hasKey(KEY_HIT_SLOP_RIGHT)) {
right = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_RIGHT))
}
if (hitSlop.hasKey(KEY_HIT_SLOP_BOTTOM)) {
bottom = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_BOTTOM))
}
if (hitSlop.hasKey(KEY_HIT_SLOP_WIDTH)) {
width = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_WIDTH))
}
if (hitSlop.hasKey(KEY_HIT_SLOP_HEIGHT)) {
height = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_HEIGHT))
}
handler.setHitSlop(left, top, right, bottom, width, height)
PixelUtil.toPixelFromDIP(hitSlop.getDouble(index))
}

handler.setHitSlop(
edge(HIT_SLOP_LEFT_IDX),
edge(HIT_SLOP_TOP_IDX),
edge(HIT_SLOP_RIGHT_IDX),
edge(HIT_SLOP_BOTTOM_IDX),
edge(HIT_SLOP_WIDTH_IDX),
edge(HIT_SLOP_HEIGHT_IDX),
)
Comment thread
coado marked this conversation as resolved.
}
}
}
Expand Down
54 changes: 31 additions & 23 deletions packages/react-native-gesture-handler/apple/RNGestureHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,24 @@ - (RNGestureHandler *)gestureHandler

static RNGHHitSlop RNGHHitSlopEmpty = {NAN, NAN, NAN, NAN, NAN, NAN};

#define RNGH_HIT_SLOP_GET(key) (prop[key] == nil ? NAN : [prop[key] doubleValue])
// `hitSlop` reaches the native side already normalized by JS into
// `[left, top, right, bottom, width, height]`, where `null` marks an unspecified edge.
typedef NS_ENUM(NSUInteger, RNGHHitSlopIndex) {
RNGHHitSlopIndexLeft = 0,
RNGHHitSlopIndexTop,
RNGHHitSlopIndexRight,
RNGHHitSlopIndexBottom,
RNGHHitSlopIndexWidth,
RNGHHitSlopIndexHeight,
RNGHHitSlopIndexCount,
};

static CGFloat RNGHHitSlopEdge(NSArray *hitSlop, RNGHHitSlopIndex index)
{
id value = hitSlop[index];
return [value isKindOfClass:[NSNumber class]] ? [value doubleValue] : NAN;
}

#define RNGH_HIT_SLOP_IS_SET(hitSlop) \
(!isnan(hitSlop.left) || !isnan(hitSlop.right) || !isnan(hitSlop.top) || !isnan(hitSlop.bottom))
#define RNGH_HIT_SLOP_INSET(key) (isnan(hitSlop.key) ? 0. : hitSlop.key)
Expand Down Expand Up @@ -166,32 +183,23 @@ - (void)updateConfig:(NSDictionary *)config
_cancelsJSResponder = [RCTConvert BOOL:prop];
}

// A cleared hit slop arrives as six unset slots rather than as `null`, because the TurboModule
// bridge drops null-valued keys on this platform. A missing key still means the property was not
// part of this update, and leaves the previous value alone.
prop = config[@"hitSlop"];
if ([prop isKindOfClass:[NSNumber class]]) {
_hitSlop.left = _hitSlop.right = _hitSlop.top = _hitSlop.bottom = [prop doubleValue];
Comment thread
coado marked this conversation as resolved.
} else if ([prop isKindOfClass:[NSNull class]]) {
// A uniform hit slop stays a plain number on the wire, skipping the array wrapper.
_hitSlop = RNGHHitSlopEmpty;
_hitSlop.left = _hitSlop.right = _hitSlop.top = _hitSlop.bottom = [prop doubleValue];
} else if ([prop isKindOfClass:[NSArray class]]) {
_hitSlop.left = RNGHHitSlopEdge(prop, RNGHHitSlopIndexLeft);
_hitSlop.top = RNGHHitSlopEdge(prop, RNGHHitSlopIndexTop);
_hitSlop.right = RNGHHitSlopEdge(prop, RNGHHitSlopIndexRight);
_hitSlop.bottom = RNGHHitSlopEdge(prop, RNGHHitSlopIndexBottom);
_hitSlop.width = RNGHHitSlopEdge(prop, RNGHHitSlopIndexWidth);
_hitSlop.height = RNGHHitSlopEdge(prop, RNGHHitSlopIndexHeight);
} else if (prop != nil) {
_hitSlop.left = _hitSlop.right = RNGH_HIT_SLOP_GET(@"horizontal");
_hitSlop.top = _hitSlop.bottom = RNGH_HIT_SLOP_GET(@"vertical");
_hitSlop.left = RNGH_HIT_SLOP_GET(@"left");
_hitSlop.right = RNGH_HIT_SLOP_GET(@"right");
_hitSlop.top = RNGH_HIT_SLOP_GET(@"top");
_hitSlop.bottom = RNGH_HIT_SLOP_GET(@"bottom");
_hitSlop.width = RNGH_HIT_SLOP_GET(@"width");
_hitSlop.height = RNGH_HIT_SLOP_GET(@"height");
if (isnan(_hitSlop.left) && isnan(_hitSlop.right) && !isnan(_hitSlop.width)) {
RCTLogError(@"When width is set one of left or right pads need to be defined");
}
if (!isnan(_hitSlop.width) && !isnan(_hitSlop.left) && !isnan(_hitSlop.right)) {
RCTLogError(@"Cannot have all of left, right and width defined");
}
if (isnan(_hitSlop.top) && isnan(_hitSlop.bottom) && !isnan(_hitSlop.height)) {
RCTLogError(@"When height is set one of top or bottom pads need to be defined");
}
if (!isnan(_hitSlop.height) && !isnan(_hitSlop.top) && !isnan(_hitSlop.bottom)) {
RCTLogError(@"Cannot have all of top, bottom and height defined");
}
_hitSlop = RNGHHitSlopEmpty;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,16 @@ - (NSDictionary *)buildManagedHandlerConfig:(const RNGestureHandlerButtonProps &
// and the handler keeps its unset default instead of hit-testing against an identical frame.
if (props.gestureHitSlop.top != 0 || props.gestureHitSlop.left != 0 || props.gestureHitSlop.bottom != 0 ||
props.gestureHitSlop.right != 0) {
config[@"hitSlop"] = @{
@"top" : @(props.gestureHitSlop.top),
@"left" : @(props.gestureHitSlop.left),
@"bottom" : @(props.gestureHitSlop.bottom),
@"right" : @(props.gestureHitSlop.right),
};
// Matches the normalized `[left, top, right, bottom, width, height]` layout the JS side sends;
// the button only exposes the four edges, so width and height are always unset.
config[@"hitSlop"] = @[
@(props.gestureHitSlop.left),
@(props.gestureHitSlop.top),
@(props.gestureHitSlop.right),
@(props.gestureHitSlop.bottom),
[NSNull null],
[NSNull null],
];
}

return config;
Expand Down
177 changes: 177 additions & 0 deletions packages/react-native-gesture-handler/src/__tests__/hitSlop.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import type { HitSlop } from '../handlers/gestureHandlerCommon';
import { normalizeHitSlop } from '../handlers/hitSlop';

describe('normalizeHitSlop', () => {
test('keeps `undefined` out of the payload and sends `null` as unset slots', () => {
// `undefined` keeps the property out of partial config updates. `null` has to
// travel as six unset slots, since the iOS bridge drops null-valued keys.
expect(normalizeHitSlop(undefined)).toBeUndefined();
expect(normalizeHitSlop(null)).toEqual([
null,
null,
null,
null,
null,
null,
]);
});

test('is idempotent', () => {
// Normalizing an already normalized value must not empty it out.
const normalized = normalizeHitSlop({ horizontal: -10, top: -5 });

expect(normalizeHitSlop(normalized)).toEqual([
-10,
-5,
-10,
null,
null,
null,
]);
expect(normalizeHitSlop(normalizeHitSlop(-10))).toBe(-10);
});

test('forwards a uniform hit slop as a plain number', () => {
// Left as a number so the bridge does not allocate an array wrapper for it;
// every platform expands it into four equal edges itself.
expect(normalizeHitSlop(-10)).toBe(-10);
expect(normalizeHitSlop(0)).toBe(0);
});

test('marks unspecified edges as `null`', () => {
expect(normalizeHitSlop({})).toEqual([null, null, null, null, null, null]);
expect(normalizeHitSlop({ left: -10 })).toEqual([
-10,
null,
null,
null,
null,
null,
]);
});

test('treats an explicitly `undefined` edge as unspecified', () => {
// Without this, the value would reach the platforms as `0` and shrink the view.
expect(normalizeHitSlop({ left: undefined, top: -5 })).toEqual([
null,
-5,
null,
null,
null,
null,
]);
});

test('expands `horizontal` and `vertical`', () => {
expect(normalizeHitSlop({ horizontal: -10 })).toEqual([
-10,
null,
-10,
null,
null,
null,
]);
expect(normalizeHitSlop({ vertical: -10 })).toEqual([
null,
-10,
null,
-10,
null,
null,
]);
expect(normalizeHitSlop({ horizontal: -10, vertical: -5 })).toEqual([
-10,
-5,
-10,
-5,
null,
null,
]);
});

test('lets an explicit edge win over the shorthand', () => {
expect(normalizeHitSlop({ horizontal: -10, left: -20 })).toEqual([
-20,
null,
-10,
null,
null,
null,
]);
expect(normalizeHitSlop({ vertical: -10, bottom: -20 })).toEqual([
null,
-10,
null,
-20,
null,
null,
]);
});

test('carries `width` and `height` through', () => {
expect(normalizeHitSlop({ left: 0, width: 20 })).toEqual([
0,
null,
null,
null,
20,
null,
]);
expect(normalizeHitSlop({ bottom: 0, height: 20 })).toEqual([
null,
null,
null,
0,
null,
20,
]);
});

test('rejects invalid `width` and `height` combinations', () => {
expect(() =>
normalizeHitSlop({ left: 0, right: 0, width: 20 } as HitSlop)
).toThrow("cannot have all of 'left', 'right' and 'width' defined");

expect(() => normalizeHitSlop({ width: 20 } as HitSlop)).toThrow(
"when 'width' is defined, either 'left' or 'right' has to be defined"
);

expect(() =>
normalizeHitSlop({ top: 0, bottom: 0, height: 20 } as HitSlop)
).toThrow("cannot have all of 'top', 'bottom' and 'height' defined");

expect(() => normalizeHitSlop({ height: 20 } as HitSlop)).toThrow(
"when 'height' is defined, either 'top' or 'bottom' has to be defined"
);
});

test('rejects a negative `width` or `height`', () => {
expect(() => normalizeHitSlop({ left: 0, width: -20 } as HitSlop)).toThrow(
"'width' cannot be negative"
);

expect(() => normalizeHitSlop({ top: 0, height: -20 } as HitSlop)).toThrow(
"'height' cannot be negative"
);
});

test('allows a zero `width` or `height`', () => {
// An empty hit area is degenerate but coherent, and a hit slop animated
// from zero upwards passes through it.
expect(normalizeHitSlop({ left: 0, width: 0 })).toEqual([
0,
null,
null,
null,
0,
null,
]);
});

test('counts a shorthand as defining both of its edges', () => {
// `horizontal` fills in both `left` and `right`, which conflicts with `width`.
expect(() =>
normalizeHitSlop({ horizontal: -10, width: 20 } as HitSlop)
).toThrow("cannot have all of 'left', 'right' and 'width' defined");
});
});
Loading
Loading