Add hitSlop prop on iOS and Android #5720

jesseruder · 2016-02-03T03:24:49Z

New prop hitSlop allows extending the touch area of Touchable components. This makes it easier to touch small buttons without needing to change your styles.

It takes top, bottom, left, and right same as the pressRetentionOffset prop. When a touch is moved, hitSlop is combined with pressRetentionOffset to determine how far the touch can move off the button before deactivating the button.

On Android I had to add a new file ids.xml to generate a unique ID to use for the tag where I store the hitSlop state. The iOS side is more straightforward.

@terribleben worked on the iOS and JS parts of this diff.

Fixes #110

facebook-github-bot · 2016-02-03T03:25:47Z

By analyzing the blame information on this pull request, we identified @dmmiller, @andreicoman11 and @Ehesp to be potential reviewers.

eslint-bot · 2016-02-03T03:56:42Z

Libraries/Components/Touchable/TouchableBounce.js

@@ -108,6 +113,10 @@ var TouchableBounce = React.createClass({
    return this.props.pressRetentionOffset || PRESS_RETENTION_OFFSET;
  },

+  touchableGetHitSlop: function(): number {


undefined This type is incompatible with number

facebook-github-bot · 2016-02-03T19:50:19Z

@jesseruder updated the pull request.

jesseruder · 2016-02-03T20:36:11Z

It looks like the tests don't like me doing import com.facebook.react.R in TouchTargetHelper. I'm not sure how to avoid this since you need to use a resource file Id or else you get an IllegalArgumentException. See http://stackoverflow.com/questions/2859574/the-key-must-be-an-application-specific-resource-id

ide · 2016-02-03T20:39:08Z

@andreicoman11 @kmagiera are you guys the best people to look at Android touch system diffs?

ide · 2016-02-03T21:09:19Z

@bestander it looks like R.java isn't generated (?) because the tests don't use Gradle -- off the top of your head do you know how to make Buck take care of this for us?

eslint-bot · 2016-02-03T23:25:07Z

Libraries/Components/Touchable/TouchableWithoutFeedback.js

@@ -118,6 +123,10 @@ var TouchableWithoutFeedback = React.createClass({
    return this.props.pressRetentionOffset || PRESS_RETENTION_OFFSET;
  },



undefined This type is incompatible with object type

facebook-github-bot · 2016-02-04T00:06:15Z

@jesseruder updated the pull request.

kmagiera · 2016-02-04T06:45:00Z

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java

+      hitSlopPixels.put("bottom", PixelUtil.toPixelFromDIP(hitSlop.getDouble("bottom")));
+      hitSlopPixels.put("left", PixelUtil.toPixelFromDIP(hitSlop.getDouble("left")));
+      hitSlopPixels.put("right", PixelUtil.toPixelFromDIP(hitSlop.getDouble("right")));
+      view.setTag(R.id.hitSlopTag, hitSlopPixels);


Why not just add an extra property to the ReactViewGroup and assing it there? Calling setTag this way will often allocate an extra SparseArray.

Makes sense. I'll change that.

facebook-github-bot · 2016-02-05T01:17:42Z

@jesseruder updated the pull request.

jesseruder · 2016-02-05T02:32:52Z

I tried adding react_native_target('java/com/facebook/react/views/view:view') to ReactAndroid/src/main/java/com/facebook/react/uimanager/BUCK but that gave me:

BUILD FAILED: Cycle found: //ReactAndroid/src/main/java/com/facebook/react/uimanager:uimanager -> //ReactAndroid/src/main/java/com/facebook/react/views/view:view -> //ReactAndroid/src/main/java/com/facebook/react/uimanager:uimanager

Is there any clean way to solve this @bestander? I could add an interface under react/uimanager but that seems like overkill.

kmagiera · 2016-02-05T08:17:10Z

ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java

-    if ((localX >= 0 && localX < (child.getRight() - child.getLeft()))
-        && (localY >= 0 && localY < (child.getBottom() - child.getTop()))) {
+    Rect hitSlopRect = new Rect();
+    if (child instanceof ReactViewGroup && ((ReactViewGroup) child).getHitSlopRect() != null) {


Can you extract getHitSlopRect to an interface so that other views (such as Text elements) can implement hitSlop too if we need it?

bestander · 2016-02-05T10:51:53Z

@jesseruder yeah, that sucks.
Looks like uimanager is quite large but I don't see a quick and clean way to split it.
Possible ways to fix: move TouchTargetHelper.java to view package or into its own package.

@mkonicek @foghina what do you think?

skevy · 2016-02-10T22:03:11Z

@mkonicek iOS side looks 👍 to me. LGTM

skevy · 2016-02-10T22:12:19Z

Also, just FYI martin, we've been using this internally and it's working great on both platforms :)

mkonicek · 2016-02-10T22:16:54Z

Oh cool, good to know!

andreicoman11 · 2016-02-11T11:19:07Z

Libraries/Components/Touchable/TouchableWithoutFeedback.js

+    /**
+     * This defines how far your touch can start away from the button. This is
+     * added to `pressRetentionOffset` when moving off of the button.
+     * The touch area never extends past the parent view bounds and the Z-index


This should be more obvious to the reader, something like ** NOTE ** or even "Experimental" would help here

facebook-github-bot · 2016-02-11T11:19:13Z

@jesseruder updated the pull request.

andreicoman11 · 2016-02-11T11:19:40Z

Libraries/Components/View/View.js

@@ -202,6 +203,14 @@ const View = React.createClass({
    onMoveShouldSetResponderCapture: PropTypes.func,

    /**
+     * This defines how far a touch event can start away from the view.
+     * The touch area never extends past the parent view bounds and the Z-index


same as above

facebook-github-bot · 2016-02-11T14:18:45Z

@jesseruder updated the pull request.

mkonicek · 2016-02-11T17:12:42Z

Libraries/Components/View/View.js

+     * This defines how far a touch event can start away from the view.
+     * The touch area never extends past the parent view bounds and the Z-index
+     * of sibling views always takes precedence if a touch hits two overlapping
+     * views.


Can you add a short example here showing how I would extend the area by n pixels in each direction? What's a reasonable number?

facebook-github-bot · 2016-02-11T17:13:14Z

@jesseruder updated the pull request.

mkonicek · 2016-02-11T17:15:18Z

Looks good! Just two small nits and would be nice to add a small example: https://github.com/facebook/react-native/blob/master/Examples/UIExplorer/TouchableExample.js

facebook-github-bot · 2016-02-11T19:19:16Z

@jesseruder updated the pull request.

jesseruder · 2016-02-11T19:19:37Z

@mkonicek @andreicoman11 Added an example and more comments. Thanks!

facebook-github-bot · 2016-02-11T19:22:17Z

@jesseruder updated the pull request.

facebook-github-bot · 2016-02-11T21:35:15Z

@jesseruder updated the pull request.

facebook-github-bot · 2016-02-15T19:40:13Z

@jesseruder updated the pull request.

facebook-github-bot · 2016-02-15T19:43:58Z

@jesseruder updated the pull request.

ide · 2016-02-16T23:26:49Z

The example looks good and this is working on both platforms.

ide · 2016-02-16T23:26:52Z

@facebook-github-bot shipit

facebook-github-bot · 2016-02-16T23:28:27Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1041390565903389/int_phab to review.

Summary:New prop `hitSlop` allows extending the touch area of Touchable components. This makes it easier to touch small buttons without needing to change your styles. It takes `top`, `bottom`, `left`, and `right` same as the `pressRetentionOffset` prop. When a touch is moved, `hitSlop` is combined with `pressRetentionOffset` to determine how far the touch can move off the button before deactivating the button. On Android I had to add a new file `ids.xml` to generate a unique ID to use for the tag where I store the `hitSlop` state. The iOS side is more straightforward. terribleben worked on the iOS and JS parts of this diff. Fixes facebook#110 Closes facebook#5720 Differential Revision: D2941671 Pulled By: androidtrunkagent fb-gh-sync-id: 07e3eb8b6a36eebf76968fdaac3c6ac335603194 shipit-source-id: 07e3eb8b6a36eebf76968fdaac3c6ac335603194

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 3, 2016

eslint-bot reviewed Feb 3, 2016
View reviewed changes

jesseruder force-pushed the feature/hit_slop branch from 779f977 to 34cec67 Compare February 3, 2016 19:49

ide added Platform: iOS iOS applications. Android and removed GH Review: review-needed labels Feb 3, 2016

eslint-bot reviewed Feb 3, 2016
View reviewed changes

jesseruder force-pushed the feature/hit_slop branch from 34cec67 to b5ee125 Compare February 4, 2016 00:05

kmagiera reviewed Feb 4, 2016
View reviewed changes

skevy added GH Review: needs-revision and removed GH Review: review-needed labels Feb 4, 2016

jesseruder force-pushed the feature/hit_slop branch from b5ee125 to a50bc13 Compare February 5, 2016 01:16

kmagiera reviewed Feb 5, 2016
View reviewed changes

jesseruder force-pushed the feature/hit_slop branch from a50bc13 to 6db6921 Compare February 5, 2016 21:22

skevy added GH Review: review-needed and removed GH Review: needs-revision labels Feb 10, 2016

andreicoman11 reviewed Feb 11, 2016
View reviewed changes

mkonicek reviewed Feb 11, 2016
View reviewed changes

jesseruder force-pushed the feature/hit_slop branch from 6db6921 to 6cdf128 Compare February 11, 2016 19:18

jesseruder force-pushed the feature/hit_slop branch from 6cdf128 to d13c51a Compare February 11, 2016 19:21

jesseruder force-pushed the feature/hit_slop branch from d13c51a to 54dcde6 Compare February 15, 2016 19:39

jesseruder force-pushed the feature/hit_slop branch from 54dcde6 to 8211582 Compare February 15, 2016 19:43

Add hitSlop prop on iOS and Android

951c8bd

jesseruder force-pushed the feature/hit_slop branch from 8211582 to 951c8bd Compare February 15, 2016 19:47

ghost closed this in 0176ac4 Feb 17, 2016

ide deleted the feature/hit_slop branch March 11, 2016 08:41

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hitSlop prop on iOS and Android #5720

Add hitSlop prop on iOS and Android #5720

jesseruder commented Feb 3, 2016

facebook-github-bot commented Feb 3, 2016

eslint-bot Feb 3, 2016

facebook-github-bot commented Feb 3, 2016

jesseruder commented Feb 3, 2016

ide commented Feb 3, 2016

ide commented Feb 3, 2016

eslint-bot Feb 3, 2016

facebook-github-bot commented Feb 4, 2016

kmagiera Feb 4, 2016

jesseruder Feb 4, 2016

facebook-github-bot commented Feb 5, 2016

jesseruder commented Feb 5, 2016

kmagiera Feb 5, 2016

bestander commented Feb 5, 2016

skevy commented Feb 10, 2016

skevy commented Feb 10, 2016

mkonicek commented Feb 10, 2016

andreicoman11 Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

andreicoman11 Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

mkonicek Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

mkonicek commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

jesseruder commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

facebook-github-bot commented Feb 15, 2016

facebook-github-bot commented Feb 15, 2016

ide commented Feb 16, 2016

ide commented Feb 16, 2016

facebook-github-bot commented Feb 16, 2016

		@@ -118,6 +123,10 @@ var TouchableWithoutFeedback = React.createClass({
		return this.props.pressRetentionOffset \|\| PRESS_RETENTION_OFFSET;
		},

Add hitSlop prop on iOS and Android #5720

Add hitSlop prop on iOS and Android #5720

Conversation

jesseruder commented Feb 3, 2016

facebook-github-bot commented Feb 3, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Feb 3, 2016

jesseruder commented Feb 3, 2016

ide commented Feb 3, 2016

ide commented Feb 3, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Feb 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Feb 5, 2016

jesseruder commented Feb 5, 2016

Choose a reason for hiding this comment

bestander commented Feb 5, 2016

skevy commented Feb 10, 2016

skevy commented Feb 10, 2016

mkonicek commented Feb 10, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Feb 11, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Feb 11, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Feb 11, 2016

mkonicek commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

jesseruder commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

facebook-github-bot commented Feb 11, 2016

facebook-github-bot commented Feb 15, 2016

facebook-github-bot commented Feb 15, 2016

ide commented Feb 16, 2016

ide commented Feb 16, 2016

facebook-github-bot commented Feb 16, 2016