add firebase in-app messaging display pod #1798
Conversation
There was a problem hiding this comment.
Update this README section and create a README - https://github.com/firebase/firebase-ios-sdk#development
| FirebaseInAppMessagingDisplay is the default UI implementation from Firebase for | ||
| rendering In-App Messaging messges to end users. | ||
|
|
||
| Apps can also provide custom UI implementation to replace it. Check out our guides |
There was a problem hiding this comment.
Add link to guides - then lgtm
There was a problem hiding this comment.
It's still being developed. Will add it once the doc is released.
|
Please wait for @ryanwilson to review before merging. |
| [self.view addGestureRecognizer:tapGestureRecognizer]; | ||
| } | ||
|
|
||
| static const CGFloat kSwipeUpThreshold = -10.0f; |
There was a problem hiding this comment.
This should likely be at the top, not inline with method implementations.
| if (self.displayDelegate) { | ||
| [self.displayDelegate messageClicked]; | ||
| } else { | ||
| FIRLogWarning(kFIRLoggerInAppMessagingDisplay, @"I-FID200008", @"Display delegate is nil"); |
There was a problem hiding this comment.
Can you elaborate a little bit more in this warning? "Firebase In App Message was tapped but the display delegate is nil - add a delegate in order to respond to actions." or something so users know what to do without having to look it up.
| @interface FIDRenderingWindowHelper : NSObject | ||
|
|
||
| // Return the singleton UIWindow that can be used for rendering modal IAM views | ||
| + (UIWindow *)uiWindowForModalView; |
There was a problem hiding this comment.
Nit: style guide-wise these should start with UI not ui. Although I think just windowForModalView seems like a better name.
There was a problem hiding this comment.
changed to UIWindow
| // Firebase InAppMessagingDisplay is merged into master. Keep them separate now to help | ||
| // with build from development folder and avoid merge conflicts. | ||
|
|
||
| // this should eventually be in FIRLogger.h |
There was a problem hiding this comment.
Nit: I think we want to eventually get constants out of FIRLogger, right @paulb777? If that's the case, please remove these comments.
There was a problem hiding this comment.
Right, we want to move the service constants to the clients
There was a problem hiding this comment.
removed the comment
|
|
||
| // Banner view will be rendered and dismissed with animation. Within viewDidLayoutSubviews function, | ||
| // we would position the view so that it's out of UIWindow range on the top so that later on it can | ||
| // be slided in with animation. However, viewDidLayoutSubviews is also triggred in other scenarios |
There was a problem hiding this comment.
so that later on it can slide in
| if (resourceBundle == nil) { | ||
| NSError *error = [NSError errorWithDomain:kFirebaseInAppMessagingDisplayErrorDomain | ||
| code:FIAMDisplayRenderErrorTypeUnspecifiedError | ||
| userInfo:@{@"message" : @"resource bundle is missing"}]; |
There was a problem hiding this comment.
Does this get presented to the user? Or just the developer?
This reverts commit c7e9b2d.
This is the firebase in-app messaging display pod, which is the default display implementation for fiam. It requires firebase in-app messaging of version 0.12.0 or higher.