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

FIAM Programmatic triggers #3081

Merged
merged 79 commits into from
May 28, 2019
Merged

FIAM Programmatic triggers #3081

merged 79 commits into from
May 28, 2019

Conversation

christibbs
Copy link
Contributor

@christibbs christibbs commented May 24, 2019

Adds public method triggerEvent: on FIRInAppMessaging to directly call on FIRIAMDisplayExecutor to check the message queue and display a contextual triggered message if applicable.

Also adds unit tests and some UI functionality on the test app to trigger this flow during manual testing.

3ca83a6 also fixes an issue where overriding the access levels of readonly properties in FIRInAppMessagingCardDisplay in a private header wasn't working as expected.

Copy link

@MeghaB MeghaB left a comment

Choose a reason for hiding this comment

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

adding myself

Copy link

@MeghaB MeghaB left a comment

Choose a reason for hiding this comment

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

API Looks good, made some logging/naming comments, and had some questions which (potentially) could just be iOS patterns, but curious :)

Copy link
Contributor

@morganchen12 morganchen12 left a comment

Choose a reason for hiding this comment

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

LGTM with one curiosity

@christibbs christibbs merged commit 93ebe6f into master May 28, 2019
@christibbs christibbs deleted the fiam-programmatic-triggers branch May 28, 2019 17:42
@firebase firebase locked and limited conversation to collaborators Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants