Questions about Touchable's onLongPress implementation and proposed changes

[jmstout]

Hi, I've been considering submitting a PR that allows the TouchableHighlight component to accept a custom delay property, addressing some of the issues discussed in #134.

In doing so I discovered some conflicts with the implementation of long press.

Currently, the long press delay is determined by LONG_PRESS_THRESHOLD - delayMS where

var LONG_PRESS_THRESHOLD = 500;

and

var delayMS =
      this.touchableGetHighlightDelayMS !== undefined ?
      this.touchableGetHighlightDelayMS() : HIGHLIGHT_DELAY_MS;

This means that if the this.touchableGetHighlightDelayMS() returns a value greater than or equal to half of the LONG_PRESS_THRESHOLD, the two will conflict with each other, resulting in a fatal error:

This got me wondering about the implementation of the long press delay.

Why is it determined by subtracting the length of the highlight delay from 500ms?

I'm not convinced this is the best interaction. It works okay for the defaults (130ms highlight delay, and long press 240ms after that, at 370ms), but in a situation where the highlight delay is longer, say 240ms, the long press will be triggered at 260ms, just 20ms after the highlight.

If anything, it seems like the long press delay should be LONG_PRESS_THRESHOLD + delayMS, where it occurs 500ms after the highlight. Though perhaps, with the current defaults, this would require a longer press duration than is desirable?

I suppose this also operates under the assumption that most people hold their finger on an element until they receive feedback, and perceive that feedback as the start of their interaction. In my mind, any long press should be contingent on the start of the initial feedback, not the start of the press.

I'm just curious about the reasoning behind the current implementation and am wondering if anyone would be opposed to changing it.

I think the Touchable components should support custom delay and longDelay properties and I am interested in submitting a PR to achieve this.

I just wanted to see if there was something I was missing before I went ahead with the changes.

Thanks! I would really appreciate any thoughts or feedback on this matter.

[vjeux]

cc @sahrens

[sahrens]

Customizing this should be fine, and I don't have a strong opinion as to the exact semantics. It would also be nice to add some clearer asserts to avoid the negative delay cases etc.

[brentvatne]

Tracking this in your PR @jmstout 😄