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

UIApplication.willEnterForegroundNotification is called each time an app enters the foreground on iOS, but only once on macCatalyst. #110

Open
michaellenaghan opened this issue Jun 29, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@michaellenaghan
Copy link

This isn't necessarily a bug, but it may be unexpected: UIApplication.willEnterForegroundNotification is called each time an app enters the foreground on iOS, but only once on macCatalyst.

https://github.com/TelemetryDeck/SwiftClient/blob/3f8bd438c8681ce7ccdf035b529cb5c4cd82cd7b/Sources/TelemetryClient/TelemetryClient.swift#L145

That means that an iOS client will generate a new session each time the app is activated, while a macCatalyst client will generate a new session each time the app is launched — even if it's left running for days, weeks or months. Again, not necessarily a bug, but someone who's focusing on session stats might think there's a lot less Mac usage than there really is.

It might also throw off TelemetryDeck's Daily Active Users and User Retention stats?

Here's a suggestion:

  • On macCatalyst, observe UIScene.willEnterForegroundNotification rather than UIApplication.willEnterForegroundNotification
  • Generate a new session whenever a) the current Y/M/D is different from the previous Y/M/D; or b) there's no previous Y/M/D

That would treat each launch and each additional day as a new session (if the app is activated by the user on that day).

@winsmith winsmith added the enhancement New feature or request label Jul 10, 2023
@winsmith
Copy link
Contributor

Love that idea!

@michaellenaghan
Copy link
Author

TL;DR

On macCatalyst, observe NSWindowDidBecomeMainNotification rather than UIScene.willEnterForegroundNotification. Since that's an NSWindow notification name, and since NSWindow notification names aren't available in macCatalyst without special effort, subscribe using the stringified name, like this:

        self.token = NotificationCenter.default.addObserver(
            forName: Notification.Name("NSWindowDidBecomeMainNotification"), 
            object: nil, 
            queue: nil
        ) {
            notification in
            ...
        }

Background

macCatalyst sends out a number of UIScene.willEnterForegroundNotification notifications — all undocumented. Here's just one example: a popover sends a UIScene.willEnterForegroundNotification notification with a _UIPopoverScene object when it's presented. Observing NSWindowDidBecomeMainNotification is much simpler (and more reliable) than figuring out which notifications to pay attention to, and which to ignore.

@winsmith
Copy link
Contributor

winsmith commented Jul 17, 2023

This is very compatible, doesn't break on other systems, and does what is needed, fantastic! I'm adding this to the roadmap – unless you'd like to submit a PR?

@michaellenaghan
Copy link
Author

Well, it depends what you mean by "break." Stats would change. Since it takes time for people to update, that change would happen over a period of time rather than all at once, making it a bit harder to "see" and reason about.

If you don't think that's a problem I think I could submit a PR? If you do, we should talk it through a bit.

The "breaking" thing is more obvious with something like #106 where old data will have the wrong values, etc. (I'm aware of yet another wrong value that I haven't reported yet.) One option: bundle this change and those changes into a bigger release with a new major version number. The question is: would there be any effort to continue to support the old behaviour, under a flag of some kind, or would there just be a clean break…?

@winsmith
Copy link
Contributor

I think that's fine. With "breaking" I meant more something like the app crashing because a notification type is not supported on macOS.

I know its been a minute, but if you're up for it I'd love the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants