[TIMOB-26294] TiAPI: Notify app about user-interactions #10248

JorenVos · 2018-08-13T12:21:06Z

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

In order to create a solution for https://jira.appcelerator.org/browse/TIMOB-13884, we're trying to capture all user interaction events. For Android, there is support to capture all onUserInteraction events. (https://jira.appcelerator.org/browse/TIMOB-26278)

We want to achieve the same functionality for iOS by extending the UIApplication and trigger an event for each UIEvent. In order to avoid negative performance impact, an event will be triggered only when a touch event began.

Test:
Run the below test on both Android and iOS.

Set up a project to use the UserInteractionAutoCloseTest.js attached to TIMOB-26294.
Build and run the app onto Android/iOS.
Tap on the "Open Child Window" button.
Note the "Will auto-close" countdown text at the top. (It starts from 10 seconds.)
Wait for the countdown to reach zero and for the child window to close.
Tap on the "Open Child Window" button.
Tap on an empty part of the window. (Verify countdown resets.)
Tap on the countdown text. (Verify countdown resets.)
Press the device's volume up/down buttons. (Verify countdown resets.)
Tap on the TextField.
Verify that countdown resets every time you enter/delete characters.
Tap on the "Show Alert" button.
On iOS, verify tapping outside of alert resets the countdown. (Not supported on Android.)

…ion and catch all UIEvents.

Master changes

…into feature/ios_catch_all_ui_events

…t to Titanium

build · 2018-08-13T12:23:56Z

	Messages
📖	🎉 Another contribution from our awesome community member, JorenVos! Thanks again for helping us make Titanium SDK better. 👍
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

hansemannn

Some changes required.

hansemannn · 2018-08-13T12:53:45Z

apidoc/Titanium/App/iOS/iOS.yml

+    description: |
+        This event will be triggered each time a UITouchPhaseBegan event is handled by the UIApplication. For more information, see https://developer.apple.com/documentation/uikit/uiapplication/1623043-sendevent
+    osver: {ios: {min: "7.0"}}
+    since: "7.4.0"


Please move the API to be a Ti.App event that can be used from Android and iOS (requires a small Android change as well). Otherwise, we have two API's for the same functionality on two namespaces. Since 7.4.0 is not released so far, the changes from #10242 could still be updated in this PR.

hansemannn · 2018-08-13T12:54:40Z

iphone/Classes/TiAppiOSProxy.m

@@ -126,6 +126,9 @@ - (void)_listenerAdded:(NSString *)type count:(int)count
                                                 name:kTiApplicationLaunchedFromURL
                                               object:nil];
  }
+  if (count == 1 && [type isEqual:@"userinteraction"]) {
+      [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(handleUserInteraction:) name:@"kTiUserInteraction" object:nil];
+    }
 }


Please move over to the Ti.App namespace. Also, lint all sources using clang-format -style=file -i Classes/<file>.m from the /iphone folder.

hansemannn · 2018-08-13T12:54:48Z

iphone/Classes/TiUIApplication.m

+            [[NSNotificationCenter defaultCenter] postNotificationName:@"kTiUserInteraction" object:nil];
+        }
+    }
+}


Also lint here.

hansemannn · 2018-08-13T12:54:59Z

support/iphone/main.m

@@ -27,7 +27,7 @@
 int main(int argc, char *argv[]) {
    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];

-	int retVal = UIApplicationMain(argc, argv, nil, @"TiApp");
+	int retVal = UIApplicationMain(argc, argv, @"TiUIApplication", @"TiApp");
    [pool release];


Please fix the indentation here.

Updated documentation

hansemannn · 2018-08-13T17:55:44Z

android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java

-		if (activityProxy != null) {
-			activityProxy.fireEvent(TiC.EVENT_USER_INTERACTION, null);
-		}
+        TiApplication.getInstance().fireAppEvent(TiC.EVENT_USER_INTERACTION, null);


Intentation here seems wrong, please also lint with clang-format -style=file -i titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java from your android/ dir.

hansemannn · 2018-08-14T20:59:35Z

iphone/Classes/TiUIApplication.m

+
+  for (UITouch *touch in event.allTouches) {
+    if (touch.phase == UITouchPhaseBegan) {
+      [[NSNotificationCenter defaultCenter] postNotificationName:@"kTiUserInteraction" object:nil];


I am starting to become unsure if we should really have this in the core. I understand the purpose for you, but this was requested by maybe 5 devs over the last years, and it must have some kind of performance hit if we post an internal notification for every tap that is made in an app.

I do agree with your concerns about potential perfomance hit. On Android, the onUserInteraction will also loop over the event listeners in order to fire an event. It can have performance impact on both platforms, but while testing it, I wasn't able to "feel"/measure it.

I also understand that on Android, we're firing an event for a native event (onUserInteraction). iOS doesn't have this event by default, so this workaround is mentioned as a solution for this problem (https://blog.gaelfoppolo.com/detecting-user-inactivity-in-ios-application-684b0eeeef5b)

In order to improve performance, there should be an if somewhere in order to only post internal notifications when there are listeners registered, but I wasn't able to find a way on how to implement this. This feature might be requested by 'only 5 devs', but I think this can't be fixed by using a module, as you need to extend the UIApplication class and register it in the main.m file of your project.

In a use case from our customer, we're in the need of implementing a way to automatically logout after x-seconds of non-activity. The way it's implemented now (firing events for user interctions) is the only way to catch user activity in Titanium, so we can handle further stuff in our Javascript code.

If we use a design that has a property (let's say trackUserInteraction) together with the userinteraction event, we could wrap the special code out using preprocessor macros on iOS. For Android, we could maybe add the handler only if the property is set or at least use it to skip the event from firing.

Thanks for your comment, but I don't have any knowledge about preprocessor macros.

On Android, I think that the event is only fired when there is a listener registered.

hansemannn · 2018-08-16T10:13:43Z

I pushed some changes to your repo e5e1bd8 to enable conditional compiling for iOS. For Android, I need a second thought from @jquick-axway if this is a suitable change or performance overhead.

hansemannn

iOS approved.

jquick-axway

CR: Pass

And thanks for changing the Android onUserInteraction() method. The performance will be better this way. The hasListeners() is a hashtable lookup, which is fine.

JorenVos · 2018-09-17T14:01:32Z

@hansemannn @jquick-axway IS this PR ready to merge?

jquick-axway · 2018-09-18T14:53:47Z

@JorenVos, this PR is currently scheduled to go into 7.5.0. The next step is that it has to go through our test team before being merged.

sgtcoolguy · 2018-09-20T18:29:23Z

apidoc/Titanium/App/App.yml

+          - iOS: See the [sendEvent](https://developer.apple.com/documentation/uikit/uiapplication/1623043-sendevent) delegate
+
+    platforms: [android, iphone, ipad]
+    since: "7.4.0"


This needs to be updated to 7.5.0 before merge now...

sgtcoolguy · 2018-09-20T18:30:03Z

apidoc/Titanium/App/App.yml

+        property to be set explicitely. For iOS, this is done to prevent additional overhead.
+    type: Boolean
+    accessors: false
+    platforms: [iphone, ipad]


Probably need to add a since value here for 7.5.0 as well

lokeshchdhry · 2018-09-20T18:52:22Z

The build fails due to :

TiBaseActivity cannot override onUserInteraction() in Activity

[javac] 	protected void onUserInteraction()

    [javac] 	               ^

    [javac]   attempting to assign weaker access privileges; was public

[javac] /Users/build/jenkins/workspace/dk_titanium_mobile_PR-10248-RZYN5DEL5MZNNA6PUJPFGGUZZPJKDYDPXTWJRVBT63CYFDHQNH6Q/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java:1498: error: cannot find symbol

[javac] 		if (TiApplication.hasListeners(TiC.EVENT_USER_INTERACTION)) {

    [javac] 		                 ^

    [javac]   symbol:   method hasListeners(String)

    [javac]   location: class TiApplication

[javac] Note: Some input files use or override a deprecated API.

[javac] Note: Recompile with -Xlint:deprecation for details.

    [javac] Note: Some input files use unchecked or unsafe operations.

    [javac] Note: Recompile with -Xlint:unchecked for details.

[javac] 2 errors

BUILD FAILED

sgtcoolguy · 2018-09-21T18:41:24Z

@hansemannn @jquick-axway The hasListeners guard call that Hans added doesn't compile as TiApplication doesn't have a static method that matches, nor an instance method if we were to try and invoke it on the instance.

It looks to me like the actual proxy that would handle the event and would have a listener is the Ti.App (AppModule). I assume we'd have to add some special code here to query the TiApplication's app event proxies to see if there's any listeners for this event...?

- Fixed compiler error in "TiBaseActivity". - Modified TextField/TextArea to fire "userinteraction" event when changing text.

jquick-axway · 2018-09-22T04:03:29Z

Updated PR:

Fixed compiler error in TiBaseActivity class.
- Turns out that fireEvent() already calls hasListeners() internally before firing. So, an extra hasListeners() call wasn't necessary.
Modified TextField/TextArea to fire "userinteraction" when adding/removing characters.

lokeshchdhry · 2018-09-25T23:27:52Z

@jquick-axway , I see for IOS. The volume buttons do not reset the counter means its does not fire the userinteraction event. Is this expected ?

jquick-axway · 2018-09-26T17:57:01Z

@lokeshchdhry, I think it's fine. There are going to be some native behavior differences between the 2 platforms and the handling needs to be reasonable enough to satisfy the use-case app developers are after, such as auto-logout after a period of non-use.

Some differences I have seen are:

On iOS, this event is fired when tapping virtual keyboard keys that do not add/remove characters such as the caps-lock, but there is no way to detect this on Android. On Android, we can only detect the text changes within a edit field.
On iOS, this event fires while an alert dialog is display, but not on Android. I think on Android, the only way to support this is to re-write our alert handling to use dialog fragments instead, but that doesn't help us if a 3rd party library uses a normal alert dialog.
On Android, we can detect all device button presses (except for the home button), but on iOS we cannot.
I haven't tested status bar swipe downs, but I suspect we cannot detect this on Android.
I forgot to test orientation changes (ie: portrait to landscape and vice-versa).

lokeshchdhry · 2018-09-26T18:39:19Z

FR Passed.

userinteraction event is fired successfully on android & IOS.
On IOS the property Ti.App.trackUserInteraction = true should be used for the userinteration event to fire.

Studio Ver: 5.1.1.201809051655
SDK Ver: 7.5.0 local build
OS Ver: 10.13.5
Xcode Ver: Xcode 9.4.1
Appc NPM: 4.2.13
Appc CLI: 7.0.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.2
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 1.8.0_161
Devices: ⇨ google Nexus 5 (Android 6.0.1)
⇨ google Pixel (Android 9)
Emulator: ⇨ Android 4.1.2
IOS devices: Iphone 7 IOS 11.4

joren-vos-aca and others added 5 commits August 10, 2018 15:27

Added TiUIApplication in order to subclass the iOS-default UIApplicat…

1918832

…ion and catch all UIEvents.

Merge pull request #1 from appcelerator/master

e4daafd

Master changes

Merge branch 'master' of https://github.com/JorenVos/titanium_mobile …

b33b541

…into feature/ios_catch_all_ui_events

Send notification and handle notification in TiAppiOSProxy, fire even…

45c13b8

…t to Titanium

iOS performance improvement to only send events on touch phase began

558c3d8

build added community 🔥 ios needs tests 🚨 labels Aug 13, 2018

Added documentation

e75a0ce

build added the docs label Aug 13, 2018

Topener requested a review from hansemannn August 13, 2018 12:53

hansemannn requested changes Aug 13, 2018

View reviewed changes

hansemannn changed the title ~~[AC-5832] - Catch UI events in iOS~~ [TIMOB-26294] iOS: Notify app about user-interactions Aug 13, 2018

hansemannn added this to the 7.4.0 milestone Aug 13, 2018

joren-vos-aca added 2 commits August 13, 2018 17:28

Moved all interactions to Ti.App namespace

b876597

Updated documentation

Fix indentation

34b4fb5

build added the android label Aug 13, 2018

hansemannn reviewed Aug 13, 2018

View reviewed changes

hansemannn added no tests and removed needs tests 🚨 labels Aug 13, 2018

hansemannn changed the title ~~[TIMOB-26294] iOS: Notify app about user-interactions~~ [TIMOB-26294] TiAPI: Notify app about user-interactions Aug 13, 2018

Lint changed files

17dd252

hansemannn reviewed Aug 14, 2018

View reviewed changes

feat: only send events if registered for

e5e1bd8

hansemannn approved these changes Aug 16, 2018

View reviewed changes

Merge branch 'master' into feature/ios_catch_all_ui_events

60d9ca8

jquick-axway self-requested a review August 16, 2018 23:36

hansemannn removed this from the 7.4.0 milestone Aug 24, 2018

hansemannn added this to the 7.5.0 milestone Aug 24, 2018

jquick-axway approved these changes Aug 28, 2018

View reviewed changes

sgtcoolguy added the in-qe-testing 🕵 label Sep 18, 2018

Merge branch 'master' into feature/ios_catch_all_ui_events

5644e96

sgtcoolguy reviewed Sep 20, 2018

View reviewed changes

sgtcoolguy assigned lokeshchdhry Sep 20, 2018

sgtcoolguy added 2 commits September 21, 2018 14:27

Set onUserInteraction() back to public

8d5a56a

Fix since values for new event/property

6dd6609

jquick-axway added 2 commits September 21, 2018 20:05

Android: Updated [TIMOB-26294] based on feedback.

01e2cf2

- Fixed compiler error in "TiBaseActivity". - Modified TextField/TextArea to fire "userinteraction" event when changing text.

Merge branch 'master' into feature/ios_catch_all_ui_events

1e0ea8f

Merge branch 'master' into feature/ios_catch_all_ui_events

bda342b

lokeshchdhry merged commit 33f2052 into tidev:master Sep 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-26294] TiAPI: Notify app about user-interactions #10248

[TIMOB-26294] TiAPI: Notify app about user-interactions #10248

JorenVos commented Aug 13, 2018 •

edited by jquick-axway

build commented Aug 13, 2018 •

edited

hansemannn left a comment

hansemannn Aug 13, 2018

JorenVos Aug 13, 2018

hansemannn Aug 13, 2018

JorenVos Aug 14, 2018

hansemannn Aug 13, 2018

JorenVos Aug 14, 2018

hansemannn Aug 13, 2018

JorenVos Aug 14, 2018

hansemannn Aug 13, 2018

JorenVos Aug 14, 2018

hansemannn Aug 14, 2018

JorenVos Aug 14, 2018 •

edited

hansemannn Aug 16, 2018

JorenVos Aug 16, 2018

hansemannn commented Aug 16, 2018

hansemannn left a comment

jquick-axway left a comment •

edited

JorenVos commented Sep 17, 2018

jquick-axway commented Sep 18, 2018

sgtcoolguy Sep 20, 2018

sgtcoolguy Sep 20, 2018

lokeshchdhry commented Sep 20, 2018

sgtcoolguy commented Sep 21, 2018 •

edited

jquick-axway commented Sep 22, 2018

lokeshchdhry commented Sep 25, 2018

jquick-axway commented Sep 26, 2018 •

edited

lokeshchdhry commented Sep 26, 2018 •

edited

[TIMOB-26294] TiAPI: Notify app about user-interactions #10248

[TIMOB-26294] TiAPI: Notify app about user-interactions #10248

Conversation

JorenVos commented Aug 13, 2018 • edited by jquick-axway

build commented Aug 13, 2018 • edited

hansemannn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JorenVos Aug 14, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented Aug 16, 2018

hansemannn left a comment

Choose a reason for hiding this comment

jquick-axway left a comment • edited

Choose a reason for hiding this comment

JorenVos commented Sep 17, 2018

jquick-axway commented Sep 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokeshchdhry commented Sep 20, 2018

sgtcoolguy commented Sep 21, 2018 • edited

jquick-axway commented Sep 22, 2018

lokeshchdhry commented Sep 25, 2018

jquick-axway commented Sep 26, 2018 • edited

lokeshchdhry commented Sep 26, 2018 • edited

JorenVos commented Aug 13, 2018 •

edited by jquick-axway

build commented Aug 13, 2018 •

edited

JorenVos Aug 14, 2018 •

edited

jquick-axway left a comment •

edited

sgtcoolguy commented Sep 21, 2018 •

edited

jquick-axway commented Sep 26, 2018 •

edited

lokeshchdhry commented Sep 26, 2018 •

edited