Add key action info to Android EventHandler events.

[aamalric]

Summary

Add event key action for Android EventHandler events, helps to know if the key event is a up (key release, value is 1) or down (key pressed, value is 0) action.

Changelog

[ANDROID] [ADDED] - Add event key action to EventHandler events.

Test Plan

No test added

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[cpojer]

👍

[cpojer]

@aamalric Before I can merge this commit, would you mind signing the CLA at code.facebook.com/cla? It only takes one minute. Thank you!

[aamalric]

@cpojer Yes for sure we will soon sign the CLA

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @aamalric in d010fc3.

^{When will my fix make it into a release? | Upcoming Releases}

[dblachut-adb]

This results in double onPress on TV devices when pressing button on a remote. Was this intended? I am guessing to since the doc was not updated: https://facebook.github.io/react-native/docs/building-for-apple-tv ?

Hmmm I may be wrong about the source of the issue but for some reason I am receiving double onPress.

[aamalric]

@dblachut-adb Hi, yes now when you press one button on TV, you'll have 2 events onPress but with 2 differents events:

• one will be for the press (eventKeyAction = ACTION_DOWN = 0)
• one will be for the release (eventKeyAction = ACTION_UP = 1).

You'll have to make the check on this event if you need to handle the press only.

[dblachut-adb]

IMHO this is not good as I would not normally expect to receive two events for single press in one handler. Correct me if I am wrong but in web environment there are two separated events for it. In non-tv RN onPress is fired once on single user touch and release, but you have an option to listen for touch down and up and other gestures separately.

As this is a breaking change and a bit counterintuitive it would be great to provide separate callbacks or if this is really desired it should be documented.

Also - do you know how is this handled on AppleTV? Didn't this commit broke the compatibility between those two platforms? I have just started to wonder...

[aamalric]

On TV you have to use the TVEventHandler wich allows you to listen to events for events not specifically 'onPress' like you said.
Take a look at https://facebook.github.io/react-native/docs/building-for-apple-tv
The TVEventHandler is enabled with a callback that will receive an event, then depending on this event parameters, it's up to you to call your own 'onPress' or 'onRelease' functions...
There's no existing mechanism of onPress/onRelease for TV inputs in RN for now as far as I know

[dblachut-adb]

Yes there is and it is specifically described in the link that you have provided. I am talking about Touchable* elements (Buttons) onPress property. Touchables can be focused using directional navigation and then after you press OK (select) on remove you will receive onPress callback.

The Touchable mixin has code added to detect focus changes and use existing methods to style the components properly and initiate the proper actions when the view is selected using the TV remote, so TouchableWithoutFeedback, TouchableHighlight, TouchableOpacity and TouchableNativeFeedback will "just work". In particular:

• onFocus will be executed when the touchable view goes into focus
• onBlur will be executed when the touchable view goes out of focus
• onPress will be executed when the touchable view is actually selected by pressing the "select" button on the TV remote.

You are talking about 'global' event handler, maybe that is where we don't understand each other. Does your change affect only global event handler?

[aamalric]

My changes were originally made to detect a press from a release on TVEventHandler events.
We don't use Touchable on our TV version as we didn't like the way the spatial navigation was handled with a TV remote...
So I must admit that I didn't test the Touchable handlers behavior with my modifications.

[dblachut-adb]

Okay sure, but if I am not mistaken this could affect the onPress of Touchables.

I have just checked and your changes are not yet published in any release
git tag --contains d010fc3bf4c4aa6a6de2fb86bc7f1d88fc5cfde4
returns nothing.

So my issue with double onPress have some different source probably (I am using latest version of RN which is 0.60.4 right now). Sorry for bothering you but I have tracked your commit and it really looked like it may be the cause.

Would you mind checking how does Touchable onPress behave with your changes? Just to make sure we will not end up with four events for some mysterious reason. ;-)

[dblachut-adb]

I can see that somebody have already filed an issue for it: #25979

[hitaloramon]

I also have the same problem. Using the mouse works fine, but using a keyboard or box control onPress runs twice

[mtkopone]

This was a pretty major breaking change for us that we did not anticipate...

[dblachut-adb]

This was a pretty major breaking change for us that we did not anticipate...

@mtkopone Are you sure that this change is the cause? As I have mentioned - it is not yet released under any version.

[hitaloramon]

Looking at the code, I think the problem is in this "OR" of print. It checks two button click actions when you press the button and when you release it.

I may be wrong, but I think the problem is because of this. I think I needed to check only 1 of the actions.

https://i.imgur.com/kawDqJ1.png

[dblachut-adb]

That was my first assumption as well, but this commit is not yet released to any RN version which means that you would have to:

• build react-native from sources
• have sources checked out to master branch or have this commit cherry picked

[freddieerg]

This thread is a year old and I still don't understand why onLongPress, onPress, onPressIn, onPressOut are handled as you would expect by touch but then onLongPress, onPressIn, onPressOut have no effect on a keypress, instead just firing onPress 3 times and expecting you to deal with it?