-
Notifications
You must be signed in to change notification settings - Fork 27
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
iOS deep linking #396
iOS deep linking #396
Conversation
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.
Have we tested this in Waypoint yet?
Per discussion at the geofence meeting, we are not going to ship |
RadarSDK/Radar.m
Outdated
if (NSClassFromString(@"XCTestCase") == nil && options.autoLogNotificationConversions) { | ||
[Radar nativeSetup]; | ||
// For most users not using these features, options be null and skipped, | ||
// For X-platform users initlizing Radar in the crossplatform layer, the optiosn will also be null as nativeSetup would had been called ealier |
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.
@KennyHuRadar Some typos in this comment. e.g., initlizing
--> initializing
, optiosn
--> options
, mixing X-platform
and crossplatform
I realize comments and release notes don't have any functional impact on the SDK, but I think it's important that we nail the details there, otherwise we look sloppy. cc @lmeier
Example/Example/AppDelegate.swift
Outdated
@@ -10,7 +10,7 @@ import UserNotifications | |||
import RadarSDK | |||
|
|||
@UIApplicationMain | |||
class AppDelegate: UIResponder, UIApplicationDelegate, UIWindowSceneDelegate, UNUserNotificationCenterDelegate, CLLocationManagerDelegate, RadarDelegate, RadarVerifiedDelegate { | |||
class AppDelegate: UIResponder, UIApplicationDelegate, UIWindowSceneDelegate, UNUserNotificationCenterDelegate, CLLocationManagerDelegate, RadarDelegate, RadarVerifiedDelegate{ |
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.
Intentional?
RadarSDK/Radar.m
Outdated
@@ -43,9 +43,10 @@ + (id)sharedInstance { | |||
return sharedInstance; | |||
} | |||
|
|||
+ (void) nativeSetup { | |||
+ (void) nativeSetup:(RadarInitializeOptions *)options { |
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.
Nit: Extra space in (void) nativeSetup
This PR adds deep linking support to the SDK.
Dashboard Setup
From the dashboard, an user should configure the geofence metadata to include the key value pair for
radar:notificationURL
SDK setup
Automatic Setup
The user can also perform the following to have setup done automatically
Manual Setup
If the user does not wish to use the automatic setup, the user should implement
Handling URL links in app
Default iOS setup
Under the hood, we will default to calling
[application openURL:url options:@{} completionHandler:nil];
You should implement
or
Developers should perform the standard iOS setup for deep linking/ universal linking.
React native setup
Dogfood/ QA:
Using a custom build of waypoint using this dependency: https://expo.dev/accounts/radarlabs/projects/waypoint/builds/37c534bf-d646-4fbb-b412-17b3cd426cca
We create a on premise notification that deeplinks to the setting page.
This feature is currently only available in the staging env
trimmed.deep.link.screen.capture.mov
Other notable changes:
We are extending the
nativeSetup
call to take in anRadarInitializeOptions
object. This is a breaking change, we need to ensure our existing customers are not impacted by this change. We are also extendingRadarInitializeOptions
to take in another flag,autoHandleNotificationDeepLinks
. There are some changes to the swizzling logic as we use it to enable more features. We perform swizzling if any of the flags for automatic setup is true. We swizzle to a method that checks ourRadarInitializeOptions
object stored inRadarSettings
to figure out what actions (handle deep links, log conversions etc) it needs to take at runtime.