TouchableHighlight vs TouchableWithoutFeedback

[jaygarcia]

I noticed a huge issue with TouchableHighlight where there is a significant delay with the onPressIn method being called, making this component next to unusable for real-time feedback.

The TouchableWithoutFeedback component, however provided realtime feedback, but being that it doesn't conform to the same component that TouchableHighlight does, styles break.

[jaygarcia]

One quick thought is I could remove nesting. Just realized that Text has onPress____ methods. Going to try that.

[vjeux]

Usually, the reason why there's a delay is that the JS thread is blocked doing something else. Do you have some timers or code running at the same time?

[jaygarcia]

No, i do not.

The strange thing is that switching components makes it lighting fast. TouchableOpacity and TouchableWithoutFeedback are extremely fast responding, while TouchableHighlight is sluggish.

[jaygarcia]

The culprit is the default HIGHLIGHT_DELAY_MS property, which is 130ms. When I change the value (see below), it works as expected.

//Libraries/vendor/react_contrib/interactions/Touchable/Touchable.js:
...
var HIGHLIGHT_DELAY_MS = 130;
...
touchableHandleResponderGrant: function(e, dispatchID) {
    // Since e is used in a callback invoked on another event loop
    // (as in setTimeout etc), we need to call e.persist() on the
    // event to make sure it doesn't get reused in the event object pool.
    e.persist();

    this.state.touchable.touchState = States.NOT_RESPONDER;
    this.state.touchable.responderID = dispatchID;
    this._receiveSignal(Signals.RESPONDER_GRANT, e);
    var delayMS =
      this.touchableGetHighlightDelayMS !== undefined ?
      this.touchableGetHighlightDelayMS() : HIGHLIGHT_DELAY_MS;
    if (delayMS !== 0) {
      this.touchableDelayTimeout = setTimeout(
        this._handleDelay.bind(this, e),
        delayMS
      );
    } else {
      this._handleDelay(e);
    }

    this.longPressDelayTimeout = setTimeout(
      this._handleLongDelay.bind(this, e),
      LONG_PRESS_THRESHOLD - delayMS
    );
  },

[jaygarcia]

The obvious question is how does one override touchableGetHighlightDelayMS for a particular configuration of TouchableHighlight? Perhaps it should be a property configuration? (Just thinking out loud)

[vjeux]

cc @sahrens who implemented the delay

[jaygarcia]

It's worth noting that the only way I could figure out how to leverage the styling of a and the speed of a Touchable is by nesting like so:

            return (
                <View style={[styles.button, styles[props.btnStyle], btnPressedStyle]}>
                    <TouchableWithoutFeedback 
                        onPress={this.onButtonPress}
                        onPressIn={this.onButtonPressIn}
                        onPressOut={this.onButtonPressOut}>
                            <Text style={styles.buttonFont}>
                                {this.buttonChars[btnChar]}
                            </Text>
                    </TouchableWithoutFeedback>
                </View>
            )

[sahrens]

I didn't implement the delay, it blames back to @jordwalke internally (D765907) and the 130ms value has remained unchanged since first introduced in react on the FB desktop website in April 2013. My guess is that it exists for reasons similar to those described here:

http://stackoverflow.com/questions/7541159/is-it-possible-to-remove-the-delay-of-uibuttons-highlighted-state-inside-a-uisc

but it doesn't really seem to be necessary since TouchableOpacity works fine.

@jaygarcia - for your last comment with the nested structure, do you still need to do that if you set the delay to zero, or does changing changing the delay fix the issue completely? We can definitely add

  touchableGetHighlightDelayMS: function() {
    return 0;
  },

to TouchableHighlight to match TouchableOpacity if that's the better behavior - want to send a PR if that fixes things for you?

[jaygarcia]

@sahrens ,

If i set the delay to zero in the following file, it works as desired, giving immediate feedback, which is what is expected.

//Libraries/vendor/react_contrib/interactions/Touchable/Touchable.js:

The nesting above isn't required if we remove the delay.

What about the other issues I surfaced, however? The fact that the other Touchable____ views not conforming to , thus preventing layout controls, etc (see above animated gif)?

Also, please pardon the newb question, but PR as in "Pull Request"? Isn't that reserved for code changes? I recall there being a Contrib's Lic. agreement that goes along with that, correct?

(This is more of a generic react question :-)
Lastly, where would someone put said override? When I require TouchableHighlight, it's a factory, from what I can tell. When I implement and place properties, those go int he props collection.

  touchableGetHighlightDelayMS: function() {
    return 0;
  },

[sahrens]

Yes, I'm suggesting that you change the implementation of TouchableHighlight.js and send us a PR with those changes (which requires the contrib agreement). We can make the change ourselves if you prefer, but it will take longer.

For the layout issues, those are due to unfortunate discrepancies in the implementations - TouchableHighlight is unique because it requires an extra view be rendered behind the content to provide the colored highlight. It's fixable, but would require some work.

[sahrens]

To get things working with TouchableWithoutFeedback, I think if you add a View wrapper around the inner text and apply the styles to that instead of to TouchableWithoutFeedback, it will work?

[jordwalke]

Yeah, the delay was intentional to mimic touchable elements that exist within scroll views. On iOS, you don't want items highlighting immediately just in case a containing scroll view begins to scroll right after the touch start. So our delay values might be too large, which we can adjust, or we could also allow override values for certain instances of your choosing.

[jaygarcia]

Allowing an override is going to be the best route, IMHO.

[jaygarcia]

@jordwalke , I'd like to invite you to install the eBay app for iOS. Their "ListView" has a delay on highlight, but immediate feedback on tap.

I believe that this level of feedback & interaction needs to be provided by react.

[jordwalke]

I thought that we did have immediate feedback on the release of a tap. You probably have to implement the onPress hook. If not, I believe that is a bug in Touchable. The real challenge which we don't yet support, is to immediately give feedback on touch start (not just release) if your button happens to not be in a scroll view.

[jaygarcia]

@jordwalke, it was not like that in a previous build. Today, it's working like a champ :)

[jmstout]

Has this been addressed? I'm experiencing the same delay, regardless of whether the TouchableHighlight component is in a ScrollView or just a View.

It's not the biggest deal, since an easy workaround is to just extend the component and override the delay:

class TouchableButton extends TouchableHighlight {
  touchableGetHighlightDelayMS() {
    return 0;
  }
}

However, I am wondering if there could be a better implementation. Currently, the stock TouchableHighlight component is noticeably unresponsive.