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

fix(braze): ensure braze has a scene #593

Merged
merged 3 commits into from
Apr 12, 2023
Merged

fix(braze): ensure braze has a scene #593

merged 3 commits into from
Apr 12, 2023

Conversation

bassrock
Copy link
Member

Summary

Ensure that braze has a scene to show the message on

Implementation Details

  • Still working to figure out why the IAM via a trigger is not showing on device.

Test Steps

  • Try the commented out code in the init with and without the windowScene code.

Screenshots

@bassrock bassrock requested a review from a team as a code owner April 12, 2023 14:19
@bassrock bassrock requested review from cyndichin and CMasterson and removed request for a team April 12, 2023 14:19
Copy link
Contributor

@cyndichin cyndichin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember that when we were testing IAM via the Braze dashboard, it worked for me without any modification to the code. Curious to understand why we would need a scene now. Also, testing steps refer to commenting in / out, but do I need to trigger an IAM from Braze too?

@bassrock
Copy link
Member Author

@cyndichin that stuff is in the comment, can you help me make it more clear? That code creates a local in app message.

@@ -108,7 +107,22 @@ extension PocketBraze: BrazeInAppMessageUIDelegate {
_ ui: BrazeInAppMessageUI,
prepareWith context: inout BrazeInAppMessageUI.PresentationContext
) {
// Customize the in-app message presentation here using the context
// We need to set our scene for Braze because our UIKit + SwiftUI stuff does not seem to work with Braze's default
Copy link
Contributor

@cyndichin cyndichin Apr 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We need to set our scene for Braze because our UIKit + SwiftUI stuff does not seem to work with Braze's default
// We need to set our scene to show our local Braze in-app messaging because our UIKit + SwiftUI stuff does not seem to work with Braze's default

@cyndichin that stuff is in the comment, can you help me make it more clear? That code creates a local in app message.

Ohh okay, added suggestion for the comment. I was more curious to understand why we need this local message when it worked previously without this code (we can also discuss live in Unleash meeting).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyndichin that i can't answer, i dug through their sdk, and cant see why. But our codes been changing a lot. This is an area where im just accepting something changed, and this fixes it. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay! thanks for always clarifying for me! approved

Copy link
Contributor

@cyndichin cyndichin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with and without the additional code, looks good!

@bassrock bassrock enabled auto-merge (squash) April 12, 2023 15:23
@bassrock bassrock merged commit 28aee3d into develop Apr 12, 2023
@bassrock bassrock deleted the braze-fix-ups branch April 12, 2023 15:36
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.

2 participants