Skip to content
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

fix(android): Ti.App wrongly fires pause/resume events when opening/closing child windows #10634

Merged
merged 7 commits into from Mar 21, 2019

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Jan 18, 2019

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

Summary:

  • Now only fires "pause" and "paused" event when app is backgrounded, like iOS.
  • Now only fires "resume" and "resumed event when app is put into the foreground (including app startup), like iOS.
  • Updated Ti.App docs to indicate Android supported these events since 7.5.0.
  • We can't use Google's ProcessLifecycleOwner Java classes until the Titanium SDK can be built with gradle. This is because we need to use Google's lifecycle compiler JAR to handle its annotations. (Alternatively, we compile with Java 8, but then we could have a potential min Android OS version issue.)

Test:

  1. Build and run the below code on Android.
  2. Tap the "Open Child" button.
  3. Observe log and verify that you do NOT see any pause/resume events.
  4. Press the "Home" button to put app in the background.
  5. Observe log and verify that you do see "pause" and "paused" events.
  6. Resume the app.
  7. Observe log and verify that you do see "resume" and "resumed" events.
  8. Tap on the "Close Child" button.
  9. Observe log and verify that you do NOT see any pause/resume events.
function appEventHandler(e) {
	Ti.API.info("@@@ '" + e.type + "' event received.");
	if (Ti.App.Android || Ti.App.Windows) {
		Ti.UI.createNotification({
			message: e.type,
			duration: Ti.UI.NOTIFICATION_DURATION_SHORT,
		}).show();
	}
}

Ti.App.addEventListener("resume", appEventHandler);
Ti.App.addEventListener("resumed", appEventHandler);
Ti.App.addEventListener("pause", appEventHandler);
Ti.App.addEventListener("paused", appEventHandler);
Ti.App.addEventListener("close", appEventHandler);

var rootWindow = Ti.UI.createWindow({ title: "Parent Window" });
var openButton = Ti.UI.createButton({ title: "Open Child" });
openButton.addEventListener("click", function() {
	var childWindow = Ti.UI.createWindow({ title: "Child Window" });
	var closeButton = Ti.UI.createButton({ title: "Close Child" });
	closeButton.addEventListener("click", function() {
		childWindow.close();
	});
	childWindow.add(closeButton);
	childWindow.open();
});
rootWindow.add(openButton);
rootWindow.open();

…ume events when opening/closing child windows

- Updated Ti.App docs to indicate Android supported these events since 7.5.0
@build
Copy link
Contributor

build commented Jan 18, 2019

Messages
📖

💾 Here's the generated SDK zipfile.

📖

✅ All tests are passing
Nice one! All 3631 tests are passing.

Generated by 🚫 dangerJS against ec7c4dc

@jquick-axway jquick-axway changed the title fix(android): Fixed bug where Ti.App wrongly fires pause/resume events when opening/closing child windows fix(android): Ti.App wrongly fires pause/resume events when opening/closing child windows Jan 18, 2019
// If no activities have been started, then app is going to be put into the foreground.
if (this.activityCount == 0) {
// Fire Ti.App resume events.
KrollModule appModule = this.tiApp.getModuleByName("App");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also put appModule in TiApplicationLifecycle, so we don't have to grab it each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that we add a new "App" module instance every time a new JavaScript runtime gets created (ie: back-out of the app and relaunch).

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: PASS

@jquick-axway
Copy link
Contributor Author

jquick-axway commented Jan 18, 2019

I was verbally told that iOS fires a "pause" event when an alert gets displayed. I'm going to double check. (Not sure if I agree with this behavior, but app devs might be designing around this.)

Edit:
I tested iOS' behavior with alert dialogs and option dialogs. iOS does not fire a pause event when dialogs are displayed. So, this PR is fine and in parity with iOS.

@ssekhri
Copy link

ssekhri commented Mar 4, 2019

The 'resume' and 'resumed' events are fired on app launch, and app foreground. They do not get fired when a window is opened.
The 'pause' and 'paused' events are fired on app background only. They do not get fired on window close.

However there is a slight difference between android and iOS related to events fired during app launch. On iOS only 'resumed' event is fired whereas for android both 'resume' and 'resumed' events are fired.

@jquick-axway
Copy link
Contributor Author

Update PR so that a "resume" event only gets fired after a "pause" event (like iOS).

You will no longer see a "resume" event on app startup... but you will still see a "resumed" event (with a 'd') on app startup.

- Changed "TiApplicationLifecycle" class' static variables to instance member variables. They don't need to be static.
@hansemannn
Copy link
Collaborator

Can this be merged soon? I see the PR is approved since a while. Thx!

@ssekhri
Copy link

ssekhri commented Mar 20, 2019

FR Passed.
The 'resumed' event is fired on app launch, and 'resume', 'resumed' events are fired on app foreground. They do not get fired when a window is opened.
The 'pause' and 'paused' events are fired on app background only. They do not get fired on window close.
Test Env:
Mac OS 10.14.3
Ti SDK: 8.1.0.v20190320134035
Appc CLI: 7.0.10
Node: 8.12.0
JDK: 9.0.4
Studio: 5.1.2.201903111843
Android Device: Pixel(v9.0), Nexus6 Emulator(v6.0)

@ssekhri ssekhri merged commit f8aa2ad into tidev:master Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants