[TIMOB-15443] Android: Allow full Activity lifecycle access for Titanium modules
GitHub Issue | n/a |
---|---|
Type | New Feature |
Priority | High |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2014-10-27T17:51:48.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 4.0.0 |
Components | Android |
Labels | android, module, module_api, notable |
Reporter | Mark Mokryn |
Assignee | Jon Alter |
Created | 2013-10-09T06:26:56.000+0000 |
Updated | 2015-03-30T21:21:07.000+0000 |
Description
The current Android module API offers access to the *application* lifecycle, but this is mostly irrelevant for Android (it's the iOS model!), where code needs access to the *Activity* lifecycle (i.e. onCreate, onResume, onPause, onStop, onDestroy, onActivityResult).
A good example is this class in the Facebook SDK: https://developers.facebook.com/docs/reference/android/current/UiLifecycleHelper
Currently, this class cannot be used in module code that runs in the context of a Window or TabGroup, since the module does not have access to that Activity's lifecycle events. This greatly complicates module porting to Titanium, since using such classes requires creating non-UI activities, and then we need methods to sync between the various Activities.
We were able to address the issues faced in TIMOB-11360 for 3.3.0, but are unable to tackle this problem this release as it required too much engineering work. We will tackle this in a subsequent release.
A general question: this is targeted for 3.5.0. What is the current status? To make it simpler: what would be good is if the proxies can access the relevant activity. For example:
Then inside the proxy module we should be able to do a setActivity(activity) (setActivity is already part of the KrollProxy API). And then add the proxy object to the activity lifecycle listeners. So really all we need is a method like ActivityProxy.getActivity(), with the tricky aspect being that the activity proxy exists before the activity is created.... As a general note, I'm not clear as to the utility of initActivity (in KrollProxy currently) since in the normal case it sets the activity to be the current activity, which would be correct only if the proxy was created while the window/tab group is in the Resumed state. Note that it is possible to hack around this by calling a proprietary method on the proxy, e.g. proxy.onResume when the window/tabgroup activity is resumed - in this case we know that TiApplication.getInstance().getCurrentActivity() provides the right activity, and we can handle what we wish. However, it is a drag to expect app developers to handle this in all their windows/tabgroups when using some modules. Bottom line - there should be a clear cut way for proxies to be associated to activities and participate in the activity lifecycle. It should be supported in the code and clearly documented in the Android module development guide. This approach makes more sense than what I wrote in the original issue, and also should be easier to implement.
Note that the workaround I mentioned above (calling a proxy method during window.activity.onResume so that getCurrentActivity is correct) requires that this PR is accepted - onResume must be called synchronously. https://github.com/appcelerator/titanium_mobile/pull/6160
An example module which needs the PR noted above, and which will be further simplified if this ticket is resolved as I noted: https://github.com/mokesmokes/titanium-android-facebook
PR: https://github.com/appcelerator/titanium_mobile/pull/6179 How to test: In Studio create an Android module with the standard template. Replace the file ExampleProxy.java with the following (change the package name if required):
Then try the module with the following test app. Watch the console for output during the activity lifecycle. Note that the proxy can be created before or after the Window or TabGroup is opened - the test app tests both cases for both Windows and TabGroups. Also tested with and without "Do not keep activities"
Note: this is not a breaking change. All existing proxies should work just fine.
I hope this PR can be merged in 3.4.1 - or are you using semver nowadays [~ingo]?
I updated the PR to add onCreate to TiLifecycle. Also updated the example above to account for this. This will break code that does not inherit from KrollProxy and uses TiLifecycle. In practice, there were only 5 files in titanium_mobile that needed the trivial fix of importing android.os.Bundle and adding an empty onCreate function. However, there may be external modules that use TiLifecycle and don't inherit from KrollProxy. Thus it's important to merge this ASAP so that developers get a heads up on the small change required. Also note that the new PR for swipeable tabs https://github.com/appcelerator/titanium_mobile/pull/6008 adds support for onSaveInstanceState and onRestoreInstanceState. So these two PRs give Titanium (and external modules) a lot more flexibility, and enable porting of 3rd party Android libraries much easier.
[~ingo] also noting that the assignee for this ticket is inactive...
[~mokesmokes] thank you. We haven't yet started Sprint 21, so that would be switched before we started. I would like to have taken this up in Sprint 20, but we need to address an issue with Google play services that came up urgently that we have to address first (TIMOB-17798)
[~ingo] I have no idea what the "sprints" are, and I get the urgency with Google Play Services causing crashes suddenly.... so yes, that requires top priority but this particular PR is very urgent too since we need this functionality to develop a new Facebook module - and that's critical too since Facebook changed a ton of stuff and their old APIs are being killed very soon. Actually it may be even worse: Facebook now requires approval for new apps seeking extended permissions, and I'm not sure if an app built with the old module will pass their review. It will be very hard, perhaps impossible to develop a solid FB module without this. Plus there is a minor breaking change here (onCreate in TiLifecycle) so best to do it as early as possible so people fix external modules before GA (that should be rare - only 5 files in titanium_mobile needed a fix) if required. 3.5.0 really must have this included.
A note regarding the PR: https://github.com/appcelerator/titanium_mobile/pull/6179 I made a design decision to reduce proxy boilerplate code to a minimum. The only thing proxies (Java obviously) need to do is implement the lifecycle methods (onCreate, onResume etc if they want to get these events) - zero boilerplate. On the Javascript side the only thing required is for the developer to pass lifecycleContainer (see the example) in the proxy creation dictionary. The downside of this approach is that if a Javascript developer passes the lifecycleContainer property to a proxy that doesn't require it, TiBaseActivity will process these events for proxies that don't need it. This has no harmful effects other than overhead, and will only occur if the Javascript developer doesn't understand what he's doing.... Two options: 1. Keep as I designed it, no proxy boilerplate code, and document that lifecycleContainer should be used (in Javascript) only for proxies that require it. (obviously this is my preference) 2. Or I can remove the changes in KrollProxy.java and proxy developers will need to add that code into each proxy they write which needs the Activity lifecycle. [~ingo] please ask your team to provide input on this design decision, I can easily modify it if required. Thanks.
Added onCreateOptionsMenu, onPrepareOptionsMenu to the interfaces in TiLifecycle. For example, I'm using this in a Chromecast module to add the Cast button to the ActionBar, from the proxy. Modules such as Facebook, Chromecast, etc really need this PR.
PR with CR fix: https://github.com/appcelerator/titanium_mobile/pull/6272 Thanks again [~mokesmokes]. You rock.
[~jalter] thanks for merge! Unfortunately though it seems you forked my branch too early, and you missed the commit for onCreateOptionsMenu and onPrepareOptionsMenu. https://github.com/mokesmokes/titanium_mobile/commit/5764a42c8d8604d17a8e994309f5293997d0b551 Can't do a Chromecast module without this :( TiBaseActivity and TiLifecycle affected, same exact pattern as all the other listeners, already being used in my app so known to work.
Guys please reopen this to add the last commit in my PR so we can forget this issue for good.... Thanks!
PR with merge conflicts resolved: https://github.com/appcelerator/titanium_mobile/pull/6275
[~jalter] thanks for the review and the work! :)
[~mokesmokes] We'll be happy to update it (cc [~bhatfield]). However, have you considered writing a blog post about your usage of it for tidev.io? I know that group of individuals is always looking for great content and this sounds like a great topic for that site.
Haha, you're more then welcome [~mokesmokes], though I guess you'll agree that the guides need to be updated as well. But those changes then might use some of the blog? Mail me at mail@tidev.io if you're interested.
I'll gladly blog about it as soon as I get my product release out ;-) Will do it soon.
3.5.X backport: https://github.com/appcelerator/titanium_mobile/pull/6650
Verified using: Mac OSX 10.10.2 Titanium SDK build: 4.0.0.v20150323131014 Titanium CLI, build: 4.0.0-alpha Alloy: 1.6.0-alpha Nexus 5 (5.0.1) Created a module and added functions that uses the activity lifecycle. It works as expected. Closing ticket.