feat: send additional Sift events on Android #8991
Conversation
|
@MounirDhahri mind pulling this down and confirming you're still seeing events correctly fired? Would be greatly appreciated 🙂 |
MounirDhahri
left a comment
MounirDhahri
left a comment
There was a problem hiding this comment.
Nice! I tested this again and it's working well. I left a minor comment then I believe we can merge this
Screen.Recording.2023-07-14.at.10.56.58.mov
MounirDhahri
left a comment
MounirDhahri
left a comment
There was a problem hiding this comment.
Sorry, I missed something before. I added it now
| SiftReactNative.setPageName(`screen_${initialRouteName}`) | ||
| SiftReactNative.upload() | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
issue(blocker): I think you need to trigger this only after the navigation is ready, otherwise it will be empty initially in case the navigation is not yet ready.
There was a problem hiding this comment.
Actually, even better, you might want to move this to be within onReady prop in the NavigationContainer to avoid watching the onReady global state
There was a problem hiding this comment.
Good catch, thank you! Updated in 3d08ba5
Description
Followup to #8370.
Sift's React Native package doesn't correctly upload events without manual calls. This means we've had a few Sift events on Android (on app open + user login), but not as many as we'd like. The recommendation from the folks who worked on it is to add these calls on navigate.
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.