-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[6_1_0][TIMOB-8430]Android: fix google issue with am/pm #9026
Conversation
f5715c5
to
e4093a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR: Pass
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. |
e8da57b
to
c624957
Compare
c624957
to
71f8738
Compare
@fmerzadyan @jquick-axway Pushed workaround for Android 5.1 - Android 6.0 |
71f8738
to
66e01ac
Compare
66e01ac
to
717edc0
Compare
717edc0
to
4407860
Compare
@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. |
@jquick-axway The This is the best way to workaround this issue. Creating a thread to poll |
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. |
Adding to the main-threads event queue every 500ms for the duration the 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 |
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 |
FR Passed. Switching between am and pm fires the Studio Ver: 4.9.0.201705170123 |
Test Case
label should show the same time, in 24 hour format, as the time picker when toggling "AM" or "PM".
JIRA: https://jira.appcelerator.org/browse/TIMOB-8430