[TIMOB-25671]Android : TextField's some returnKeyType values not firing 'return' event

[ypbnv]

JIRA: https://jira.appcelerator.org/browse/TIMOB-25671

Description:
Always trigger the return key event from software keyboard.
In Landscape mode for phones Android provides both the return key for New Line and an Action key with the desired IME_ACTION mode. I extended the return event with a property that distinguishes from which key the return event has been fired.
Names of constants and name of the property I have added can be changed.

Note: This relies on user interaction with the software keyboard, so I don't think we can have unit tests for the code.

Test case:
app.js

var win = Ti.UI.createWindow({ backgroundColor : 'gray'});

var textField = Ti.UI.createTextField({
    top : 40,
    left : 20,
    right : 20,
    hintText : 'Hit \'Return\' button'
});

var textArea = Ti.UI.createTextArea({
  font: {fontSize:20, fontWeight:'bold'},
  textAlign: 'left',
  value: 'I am a textarea',
  top: 150,
  width: 300, height : 70
});

textField.addEventListener('return', function (e) {
  showAlert(e);
});

textArea.addEventListener('return', function (e) {
  showAlert(e);
});

function showAlert(event) {
  var message;
  if (event.button == Ti.UI.BUTTON_TYPE_ACTION) {
      message = 'Action!';
  } else {
      message = 'Cartridge return!';
  }
  alert(message);
}

win.add(textField);
win.add(textArea);
win.open();

[build]

	Messages
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

[jquick-axway]

@ypbnv, I tested the "return" event handling on iOS with TextFields and TextAreas. The "return" event is always fired when the key is pressed. It is also fired in a TextAreas where the return key inserts a newline. I've tried with suppressReturn property set to true/false (an iOS-only feature) and the "return" event is always fired as well (this property just changes what the key does; insert newline or act as a submit button).

[jquick-axway]

I like the idea of indicating the return type. Especially on iOS since TextAreas can fire a "return" event when its an action or newline, depending on the TextArea.suppressReturn property setting. But I don't think we should add this feature in a patch release. We should talk to @hansemannn about it. Ideally, we should add such a feature on both Android and iOS at the same time.

Also, I think you mean "carriage return", not "cartridge return". :)
The comments below need to be correct for this as well.

And perhaps calling the return key type "NEWLINE" instead of "CARRIAGE_RETURN" would be better as well. Especially since Android and iOS technically add a \n character, not a \r.

[ypbnv]

Yes, "carriage". I guess I carrieged away typing it wrong...
If we rename the constant we can get replace it with "new line" everywhere.

Also, yes, I did not think about a patch release being inappropriate for new additions. Would a minor be fine if we push it together with iOS? For now I will comment the additional key in the dictionary. We can have a separate ticket for that if we are doing it.

[jquick-axway]

The idea is new "features" and APIs should not go into patch releases. So, if we're adding a new API, it's a new feature. And ideally, new APIs should be discussed with Hans (iOS) and Kota (Windows) in advance for their feedback and so they can schedule it to be implemented in the future.

That said, we do allow "enhancements" in a patch release from time to time. That's a gray-area for us. But typically its a change in behavior for an existing API (assuming it's not a breaking change). Any enhancements we want to add to a patch release needs to be a team decision beyond you and I.

[jquick-axway]

Instead of checking if getMaxLines() is 1, we should use the field member variable instead. It'll be set to true for TextFields.

Note that upcoming PR #9927 will make max-lines settable and I don't want you to get burned by this.

[jquick-axway]

@ypbnv, do you have a bluetooth keyboard you can test with on your Android phone? We may want to double check the handling of the return key from the keyboard as well.

[ypbnv]

@jquick-axway Yes, I can get my hands on a bluetooth keyboard to check if everything is fine.

[ypbnv]

@jquick-axway Tried a bluetooth keyboard with a tablet (Samsung Galaxy Tab S) and a phone (Samsung S8). Both in landscape and portrait. Hitting the return key on the keyboard is handled as a KeyEvent in all cases. And the result is that our Ti.UI.TextField.return and Ti.UI.TextArea.return events are fired twice. No matter if the component has returnKeyType set.
This is fixed after applying the change from #9948 together with this PR.
I suspect this is the case the customer had, because it matches the behavior with versions as reported.

[jquick-axway]

Just one more minor edit. Everything else is fine. Thanks!

[jquick-axway]

Let's get rid of this description since we're not offering this new property yet.

[lokeshchdhry]

FR Passed.

return event is successfully fired for all returnKeyType.

Studio Ver: 5.1.0
SDK Ver: 7.2.0.GA
OS Ver: 10.13.2
Xcode Ver: Xcode 9.2
Appc NPM: 4.2.12
Appc CLI: 7.0.2
Daemon Ver: 1.0.1
Ti CLI Ver: 5.0.14
Alloy Ver: 1.11.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 1.8.0_131
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Nexus 6P --- Android 8.0.0

[lokeshchdhry]

Waiting for CR to pass.