braze-inc / braze-react-native-sdk

Public repo for the Braze React Native SDK
https://www.braze.com
Other
64 stars 83 forks source link

[Bug]: Unable to import `BrazeUI` code into ObjC++ files #206

Closed Alserk closed 9 months ago

Alserk commented 1 year ago

Which Platforms?

iOS

Which React Native Version?

=0.70

Which @braze/react-native-sdk SDK version?

=4.0.0

Repro Rate

100%

Steps To Reproduce

We try to replace Appboy by Braze SDK. The function

-(enum BRZInAppMessageUIDisplayChoice)inAppMessage:(BrazeInAppMessageUI *)ui
                            displayChoiceForMessage:(BRZInAppMessageRaw *)message  {
  NSData *inAppMessageData = [message json];
  NSString *inAppMessageString = [[NSString alloc] initWithData:inAppMessageData encoding:NSUTF8StringEncoding];
  NSDictionary *arguments = @{
    @"inAppMessage" : inAppMessageString
  };
  // Send to JavaScript
  [self.bridge.eventDispatcher
             sendDeviceEventWithName:@"inAppMessageReceived"
             body:arguments];

  // If the feed_type key is present in extras we discard the Braze SDK UI and allow this to be handled in our RN App.
  // Otherwise the message will be displayed using the default Braze SDK UI.
  return inAppMessage.extras[@"feed_type"] ? BRZInAppMessageUIDisplayChoiceDiscard : BRZInAppMessageUIDisplayChoiceNow;
}

should be used instead of

- (ABKInAppMessageDisplayChoice) beforeInAppMessageDisplayed:(ABKInAppMessage *)inAppMessage {
  NSData *inAppMessageData = [inAppMessage serializeToData];
  NSString *inAppMessageString = [[NSString alloc] initWithData:inAppMessageData encoding:NSUTF8StringEncoding];
  NSDictionary *arguments = @{
    @"inAppMessage" : inAppMessageString
  };
  // Send to JavaScript
  [self.bridge.eventDispatcher
             sendDeviceEventWithName:@"inAppMessageReceived"
             body:arguments];

  // If the feed_type key is present in extras we discard the Braze SDK UI and allow this to be handled in our RN App.
  // Otherwise the message will be displayed using the default Braze SDK UI.
  return inAppMessage.extras[@"feed_type"] ? ABKDiscardInAppMessage : ABKDisplayInAppMessageNow;
}

as stated in doc https://github.com/braze-inc/braze-docs/blob/develop/_docs/_developer_guide/platform_integration_guides/react_native/inapp_messages.md.

But the current SDK doesn't allow import BrazeUI inside AppDelegate file.

Expected Behavior

#import <BrazeUI/BrazeUI-Swift.h> should work as well as #import <BrazeKit/BrazeKit-Swift.h>

Actual Incorrect Behavior

Can't import BrazeUI

Verbose Logs

No response

Additional Information

Demo repo is here https://github.com/Alserk/BrazeTest

jerielng commented 1 year ago

Hi @Alserk thanks for the sample repo - we'll take a look to fully understand the importing issue and keep you posted.

jerielng commented 1 year ago

@Alserk Our BrazeUI library is exported and published in a way that doesn't make it available via a -Swift.h module header file, unfortunately. We'll continue to explore different options that will make this available in the React Native context.

In the meantime, there is one path forward that may be worth trying. If possible, you could encapsulate the BrazeUI logic you wish to use in a regular Objective-C file instead of Objective-C++. In an Objective-C environment, you can leverage the @import BrazeUI; syntax, which would give you access to the BrazeUI library. You may run into some issues if you needed to return and access some of the BrazeUI symbols in your Objective-C++ file, but if your use case allows, you may be able to use that approach to accomplish this. Let us know if that works for you!

cam-shaw commented 1 year ago

@jerielng this is a fairly significant breaking point for this SDK as by default React Native uses Objective-C++ for its AppDelegate file. It could be handy to have a notice somewhere in the docs to say it's currently not supported for AppDelegate.mm files.

Is there any chance the team could come up with an example for how to achieve your suggestion? Having a play myself, but I'm a rookie with obj-c 😅

Is there a possible workaround here where we can use the JS side to throw back to the native side?

e.g.

 Braze.subscribeToInAppMessage(false, (event) => {
      const inAppMessage = new Braze.BrazeInAppMessage(event.inAppMessage);
      if (inAppMessage.extras['isPotato']) {
        // handle on JS side
      }
      // else return something to the native side to use native UI
    });

The issue with this approach of course is the first param to subscribeToInAppMessage determines whether to show the native UI. Is there a workaround to this? It would be cool if there was a way we could conditionally trigger the native UI from the JS side.

jerielng commented 1 year ago

Hey @cam-shaw thanks for the feedback! Our internal team will continue exploring compilation options with CocoaPods to make the integration process with BrazeUI more seamless in React Native.

We have some code snippets in the works on the Objective-C layer and can share those when we are able to verify a working solution.

That being said, could you shed more insight on what you would like to accomplish with the JS side communicating back to the native UI? For more context, that useBrazeUI parameter is a flag to disable the default Braze IAM presenter in case you would like to implement your own UI layer.

cam-shaw commented 1 year ago

@jerielng The thought was, if we can call something such as a Braze.displayNativeIAM(iam) function, it would allow us to control from the JS side whether we want the native side to handle the IAM.

The downside to my idea is the IAM would come across the bridge to the JS side, and then to be shown natively again, it would be passed back across the bridge to the native side. The upside here is that it does allow for controlling whether to show the native IAM in one place instead of having to implement code natively on both platforms.

Further example

In App.tsx

// Calling false on this function would prevent all IAM from being displayed natively and would instead send it across to the JS side by default.
// Pretty much exactly how `this.bridge.subscribeToInAppMessage(false);` works
Braze.handleIAMNatively(false);
Braze.subscribeToInAppMessage(false, (event) => {
  const inAppMessage = new Braze.BrazeInAppMessage(event.inAppMessage);
  if (inAppMessage.extras["flagFromKeyValues"]) {
    // handle on JS side. Consumer can do anything here.
    return;
  }
// Otherwise, we can have the ability to send back over the bridge to tell the native SDK to handle this IAM natively.
  Braze.displayNativeIAM(iam);
});

My example would require adding the two functions, handleIAMNatively and displayNativeIAM.

hokstuff commented 1 year ago

Hi @cam-shaw,

We are trying to better understand your use case around needing to separate between displaying IAMs in the JS layer vs native layer. We expose the IAM to the JS layer by sending the data model over from our iOS bridge to the JS layer. Also, as noted above, you can leverage the parameter useBrazeUI to determine whether or not our default UI is used to display the IAM (in case you are already handling displaying it in the JS layer of your application). From what it sounds like from above, you should be able to achieve the requested functionality with our existing feature set without needing to complicate our public interface. Let me know if I'm missing any context here!

Regarding importing BrazeUI in the AppDelegate, can you describe your use case you are trying to achieve? Clang itself does not support the @import syntax for C++. As of now, we are looking into a longer term solution to expose the UI symbols via an umbrella header or some other way to be used directly in ObjC++ code. In the meantime, there appears to be possible workarounds like mentioned above which may be able to bring in those UI symbols to be used in ObjC++ files.

Thanks!

jerielng commented 9 months ago

Hi @Alserk @cam-shaw, just a quick update here:

Starting with 7.4.0 of our underlying native Swift SDK bindings, you should be able to import BrazeUI as a static XCFramework from this repository by overriding the import source URLs in your Podfile. For example, you can do the following:

pod 'BrazeKit', :podspec => 'https://raw.githubusercontent.com/braze-inc/braze-swift-sdk-prebuilt-static/{your-version}/BrazeKit.podspec'
pod 'BrazeUI', :podspec => 'https://raw.githubusercontent.com/braze-inc/braze-swift-sdk-prebuilt-static/{your-version}/BrazeUI.podspec'
pod 'BrazeLocation', :podspec => 'https://raw.githubusercontent.com/braze-inc/braze-swift-sdk-prebuilt-static/{your-version}/BrazeLocation.podspec'

Importing our Swift SDK as a prebuilt static XCFramework should allow you to import BrazeUI like so into an Objective-C++ file:

#import <BrazeUI/BrazeUI-Swift.h>

This should now be achievable in our 8.3.0 React Native SDK release since we've updated our Swift SDK bindings to the latest version. Please let us know if you have any further questions. Thanks!

cam-shaw commented 9 months ago

Thanks @jerielng! When I come around to this task again I'll be able to update if we have any issues. Regarding your comments from April 28, and excuse me because it's been a while since I was last thinking of this 😅

The goal I was trying to achieve was having a IAM listener on the JS side (.subscribeToInAppMessage) which didn't need the useBrazeUI boolean provided in the first argument.

The issue with providing the useBrazeUI boolean as the first argument for the subscribe function is we can't use any of the callback payload data to determine if we want to use Braze UI or not.

As in the original issue description, on the native side we are able to look at the payload and then choose which UI we want to use - ideally we'd have the ability to do this on the JS side as well.

Note: My suggestion originated as a workaround for the issue, but is now perhaps more of a feature request.

From OP message:

// Send to JavaScript
  [self.bridge.eventDispatcher
             sendDeviceEventWithName:@"inAppMessageReceived"
             body:arguments];

  // If the feed_type key is present in extras we discard the Braze SDK UI and allow this to be handled in our RN App.
  // Otherwise the message will be displayed using the default Braze SDK UI.
  return inAppMessage.extras[@"feed_type"] ? ABKDiscardInAppMessage : ABKDisplayInAppMessageNow;
jerielng commented 9 months ago

Got it, @cam-shaw, thanks! With regards to your in-app message display, it sounds like you should be able to simply use the .addListener call instead of the subscribe method. We refined a lot of this behavior in 6.0.0 to be more streamlined and to accommodate more nuanced use cases (docs here).

jerielng commented 9 months ago

Hi all, in light of us releasing compatibility with prebuilt static XCFrameworks per this comment, this request should now be achievable in the Braze React Native SDK. I'll go ahead and close out this issue, but if you have further questions or requests along this topic, feel free to open a new issue with more details.

Thanks!