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

iOS deep linking #396

Merged
merged 32 commits into from
Nov 14, 2024
Merged

iOS deep linking #396

merged 32 commits into from
Nov 14, 2024

Conversation

KennyHuRadar
Copy link
Contributor

@KennyHuRadar KennyHuRadar commented Oct 1, 2024

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

image

SDK setup

Automatic Setup

The user can also perform the following to have setup done automatically

radarInitializeOptions.autoHandleNotificationDeepLinks = true
Radar.initialize(publishableKey: "prj_test_pk_000", options: radarInitializeOptions )

Manual Setup

If the user does not wish to use the automatic setup, the user should implement

    func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse) async {
        Radar.openURLFromNotification(response.notification)
    }

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

func application(_ app: UIApplication, open url: URL,
                     options: [UIApplication.OpenURLOptionsKey : Any] = [:]) -> Bool

or

func scene(_ scene: UIScene, 
           willConnectTo session: UISceneSession, 
           options connectionOptions: UIScene.ConnectionOptions)

Developers should perform the standard iOS setup for deep linking/ universal linking.

React native setup

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  self.moduleName = @"main";

  // You can add your custom initial props in the dictionary below.
  // They will be passed down to the ViewController used by React Native.
  self.initialProps = @{};

  BOOL res = [super application:application didFinishLaunchingWithOptions:launchOptions];
  RadarInitializeOptions *radarInitializeOptions = [[RadarInitializeOptions alloc] init];
  radarInitializeOptions.autoHandleNotificationDeepLinks = YES;
  [Radar nativeSetup:radarInitializeOptions];
  return res;
}

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 an RadarInitializeOptions object. This is a breaking change, we need to ensure our existing customers are not impacted by this change. We are also extending RadarInitializeOptions 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 our RadarInitializeOptions object stored in RadarSettings to figure out what actions (handle deep links, log conversions etc) it needs to take at runtime.

@KennyHuRadar KennyHuRadar changed the title iOS deep linking [WIP] iOS deep linking Oct 1, 2024
@KennyHuRadar KennyHuRadar marked this pull request as ready for review October 2, 2024 14:32
@KennyHuRadar KennyHuRadar changed the title [WIP] iOS deep linking iOS deep linking Oct 2, 2024
Copy link
Contributor

@lmeier lmeier left a 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?

RadarSDK/RadarNotificationHelper.m Outdated Show resolved Hide resolved
RadarSDK/RadarNotificationHelper.m Show resolved Hide resolved
@KennyHuRadar
Copy link
Contributor Author

Per discussion at the geofence meeting, we are not going to ship RadarURLDelegate, developers should follow the standard deep linking setup for native and iOS to setup this feature.

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
Copy link
Contributor

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

@@ -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{
Copy link
Contributor

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 {
Copy link
Contributor

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

@KennyHuRadar KennyHuRadar merged commit 6632c05 into master Nov 14, 2024
2 checks passed
@KennyHuRadar KennyHuRadar deleted the kenny/deep-linking-spike branch November 14, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants