[6_1_0][TIMOB-8430]Android: fix google issue with am/pm

[frankieandone]

Test Case
label should show the same time, in 24 hour format, as the time picker when toggling "AM" or "PM".

var win = Titanium.UI.createWindow();
var lbl = Titanium.UI.createLabel({
	color: '#999',
	text: 'Set the time below',
	font: {fontSize: 20, fontFamily: 'Helvetica Neue'},
	textAlign: 'center',
	width: 'auto',
	top: '0'
});
var pkr = Titanium.UI.createPicker({
	type: Ti.UI.PICKER_TYPE_TIME,
	top: '40',
	backgroundColor: "red",
	format24: false
});
pkr.addEventListener('change', function (e) {
	lbl.text = pkr.value;
	console.log("test msg picker value: " + pkr.value);
});
win.add(lbl);
win.add(pkr);
win.open();

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

[jquick-axway]

CR: Pass

[lokeshchdhry]

FR Failed.

On Android 5.1.1. & 6.0.1 the change event does not fire when I change From AM to PM & vice versa. But, if you change from e.g AMtoPM, I have to tap on the hour or minute for the change event to fire.

[garymathews]

@fmerzadyan @jquick-axway Pushed workaround for Android 5.1 - Android 6.0

[jquick-axway]

@garymathews, I'm not comfortable with us accessing undocumented things like this (ie: "android:id/am_label" and "android:id/pm_label"). We don't know what'll happen on forked Android OS versions.

Let's stick to Frankie's changes. It's a bit more complicated, but it uses public APIs.

[garymathews]

@jquick-axway The id won't change, those versions are set in stone. If for whatever reason someone forks Android and decides to change the resources of the official android.widget.TimePicker then the code will not implement the workaround.

This is the best way to workaround this issue. Creating a thread to poll TimePicker for a value change is a really bad idea.

[jquick-axway]

Frankie's solution isn't using a "Timer" thread anymore. He switched it over to use a "Handler" which posts a Runnable on the main thread's event queue. It's safe.

[garymathews]

Adding to the main-threads event queue every 500ms for the duration the TimePicker is alive is a bad idea too. This means there could be up-to a 500ms delay from when the user presses AM/PM to when change event is fired. This could also cause unwanted race-conditions if a user is modifying the TimePicker within this time-frame programmatically, this would become our bug.

Adding a listener to a resource that we know isn't going to change is safer. The worst case scenario is that someone is running an unofficial modified fork of Android (which we don't support anyway) that happens to also have a modified TimePicker core component. In which case, the workaround won't be implemented and default behaviour will persist.

[sgtcoolguy]

I strongly prefer this simpler approach that the one used in #8799. I think if/when this passes FR, we should merge this and cherry-pick to master.

However, I do think we should incorporate some of the code cleanup from #8799 on master branch where the string literals were replaced with proper constant references for property and event names (such as the changes to android/modules/ui/src/java/ti/modules/titanium/ui/PickerProxy.java there)

[lokeshchdhry]

FR Passed.

Switching between am and pm fires the change event successfully every time.

Studio Ver: 4.9.0.201705170123
SDK Ver: 6.1.0.v20170518131826
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.2
Appc NPM: 4.2.9
Appc CLI: 6.2.1
Ti CLI Ver: 5.0.13
Alloy Ver: 1.9.11
Node Ver: 6.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 6 --- Android 6.0.1
⇨ google Pixel XL --- Android 7.1.1