-
-
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
[TIMOB-15443] Android: Enable proxies to get window/tabgroup activity lifecycle events #6179
Conversation
@@ -394,9 +394,6 @@ protected void handlePostOpen() | |||
// Prevent any duplicate events from firing by marking | |||
// this group has having focus. | |||
isFocused = true; | |||
|
|||
// Setup the new tab activity like setting orientation modes. | |||
onWindowActivityCreated(); |
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.
The call to onWindowActivityCreated
is redundant since it's already called in TiBaseActivity.onCreate for both Windows and TabGroups. That function is not idempotent thus it's a bug.
The TiLifecycle is missing the onCreate method. In order to not break existing proxies, I did not add it...... but I'm not sure why we don't have the full lifecycle states. Can someone please shed some light on this? Perhaps we should bite the bullet and add all the callbacks, fix any issues in the main source, and assume external modules will be fixed by their authors? In any case, at this point this PR is not a breaking change. |
@ingo, would love to have this so the new Facebook Android module is more user friendly. |
@FokkeZB @ingo not just for user friendliness - at this point we don't have a Facebook module without this PR or #6160 or both. This one is more elegant and easier to use, plus since @pingwang2011 expressed concerns about making synchronous calls to Javascript during the activity lifecycle perhaps we should remove those callbacks entirely (as async calls they are often mistimed with the actual events) - and people who need them can just use the API in this PR in their module. |
To reiterate: I actually think it's important that we add onCreate to the Java lifecycle callbacks. Any proxies that inherit KrollProxy do not need to be modified. It will require only 4 or 5 small modifications in the code base (just adding an empty function does it). It may possibly break external modules that don't inherit from KrollProxy but that bug will show up in compile time and is trivial to fix. I strongly suggest we add onCreate, unless there are good reasons not to. |
OK.... added onCreate to TiLifecycle. The change broke only 5 files that did not inherit from KrollProxy. This is a breaking change and could affect external modules that don't inherit from KrollProxy and use TiLifecycle. Fixing them is trivial - just import android.os.Bundle and adding an empty onCreate function, but it's important that developers get a heads up and thus please merge this ASAP. Also note that with this PR and #6008 Titanium code (and modules) will have access to the full Activity lifecycle including onSave/onRestoreInstanceState. This will make porting Android libraries much much easier. |
…ding TiActivitySupportHelper
The PR has also been updated to include onActivityResult events (this is a backwards compatible change, no effect on existing code). It is possible for modules to use TiActivitySupportHelper for this functionality, but this is a clumsy mechanism if we're porting 3rd party libraries that launch their own activities (e.g. Facebook SDK which launches the Facebook app for authentication, permissions, etc.). Adding this mechanism directly enables easy usage of onActivityResult without needing to hack the 3rd party SDK to use TiActivitySupportHelper. To use this functionality the proxy can call |
…te) to TiLifecycle
Added saveInstanceState and restoreInstanceState. This does not affect existing code. This particular addition is already in this PR: #6008 but I needed it here for the Facebook module. Depending upon which PR is merged first I will fix the other one to make it mergeable. To see all these APIs in action see this file in the new Facebook module: https://github.com/mokesmokes/titanium-android-facebook/blob/master/src/com/ti/facebook/ActivityWorkerProxy.java |
@@ -512,6 +513,7 @@ public void onResume(Activity activity) | |||
public void onDestroy(Activity activity) {} | |||
public void onStart(Activity activity) {} | |||
public void onStop(Activity activity) {} | |||
public void onCreate(Activity activity, Bundle savedInstanceState) {} |
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.
To comply with the updated interface since onCreate was added
…vailable for modules/proxies
Added onCreate/PrepareOptionsMenu interfaces. An example of this is when developing the Chromecast module, I need access to onCreateOptionsMenu in the proxy to add the Cast button to the ActionBar. |
This was merged by #6272 so should be closed.... |
Closing as requested by @mokesmokes |
Very simple to use:
lifecycleContainer: windowOrTabGroup
See example in JIRA https://jira.appcelerator.org/browse/TIMOB-15443