-
Notifications
You must be signed in to change notification settings - Fork 394
[Review Only] Google Analytics changes #1146
base: v.next
Are you sure you want to change the base?
Changes from all commits
4381c50
1b2b399
b7bda48
84424ed
a959b2d
a229926
4434568
5a35dc4
f0f9d18
0c8ceba
2df8975
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
|
||
import UIKit | ||
import ArcGIS | ||
import Firebase | ||
|
||
@UIApplicationMain | ||
class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDelegate { | ||
|
@@ -48,6 +49,9 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele | |
// Enable/disable touches based on settings. | ||
self.setTouchPref() | ||
|
||
// Set up Google Analytics 4. | ||
self.setGoogleAnalyticsPref() | ||
|
||
// Set license key and/or API key. | ||
application.license() | ||
|
||
|
@@ -94,6 +98,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele | |
} | ||
} | ||
|
||
// MARK: - Google Analytics settings | ||
|
||
func setGoogleAnalyticsPref() { | ||
// Enable/disable GA based on settings. | ||
let disableGoogleAnalytics = UserDefaults.standard.bool(forKey: "disableGoogleAnalytics") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is due to an iOS limitation, see: https://stackoverflow.com/questions/36253998 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't an iOS limitation but rather a misunderstanding of the User Defaults API. It has been a while since I've used it, but if I remember quickly, default values must be registered using |
||
if !disableGoogleAnalytics { | ||
FirebaseApp.configure() | ||
} | ||
} | ||
|
||
// MARK: - Appearance modification | ||
|
||
func modifyAppearance() { | ||
|
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.
I've never used Firebase before. What impact does adding this dependency have on app size (how large is the Firebase framework)? Also, will we need to change the App Privacy section of our App Store page?
For what it is worth, I generally am not in favor of adding third part dependencies unless there is an extremely compelling reason to do so. Each dependency, especially one like this that executes code even before that app is useable by the user, is a failure point. Remember a while back when a bunch of prominent apps stopped working for a while because they used a popular third party analytics framework that had a problem? That is the power that dependencies like this hold. In my opinion, it isn't worth it. But that's up to you and the samples team to decide.
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 impact is quite minimum. The previous version (100.13.0.1) of sample viewer app is ~85 MB (can't remember exact number), and the current version (100.13.0.2) is 87 MB, according to App Store on iPhone 11.
I've included usage data (select sample, category, and source code view) and search history (search bar) policies, prior to the release.
I agree. I also tried to avoid using 3rd party dependencies in my personal project.
We researched a few options: Google Analytics, Amazon Pinpoint, Adobe Analytics, and building an AWS based in house solution. Eventually Trevor decided GA is the easiest way to go for now, as we wanted to use it as an experiment to see what data we can get. In the future we may decide to move to other solutions or remove GA dependency.