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.

Sign up for GitHub

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

feat: send additional Sift events on Android #8991

Merged
merged 5 commits into from
Jul 17, 2023
Merged

feat: send additional Sift events on Android #8991

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0af6ce
feat: send additional Sift events on Android
mdole Jul 13, 2023
105f303
fix: correctly set initial route name
mdole Jul 14, 2023
70fb134
chore: patch-package for types
mdole Jul 14, 2023
014c20d
chore: update hacks.md
mdole Jul 14, 2023
3d08ba5
fix: set route name onReady
mdole Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/app/store/config/features.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ export const features: { [key: string]: FeatureDescriptor } = {
readyForRelease: false,
showInDevMenu: true,
},
AREnableAdditionalSiftAndroidTracking: {
mdole marked this conversation as resolved.
Show resolved Hide resolved
description: "Send additional events to Sift on Android",
readyForRelease: true,
showInDevMenu: true,
echoFlagKey: "AREnableAdditionalSiftAndroidTracking",
},
}

export interface DevToggleDescriptor {
Expand Down
27 changes: 27 additions & 0 deletions src/app/system/navigation/ModalStack.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import { Severity, addBreadcrumb } from "@sentry/react-native"
import { AppModule, modules } from "app/AppRegistry"
import { __unsafe_mainModalStackRef } from "app/NativeModules/ARScreenPresenterModule"
import { GlobalStore } from "app/store/GlobalStore"
import { useFeatureFlag } from "app/utils/hooks/useFeatureFlag"
import { logNavigation } from "app/utils/loggers"
import { useEffect, useRef } from "react"
import { Platform } from "react-native"
import SiftReactNative from "sift-react-native"
import { NavStack } from "./NavStack"
import { useReloadedDevNavigationState } from "./useReloadedDevNavigationState"

Expand All @@ -24,6 +27,24 @@ const Stack = createStackNavigator()
export const ModalStack: React.FC = ({ children }) => {
const initialState = useReloadedDevNavigationState("main_modal_stack", __unsafe_mainModalStackRef)
const { setSessionState: setNavigationReady } = GlobalStore.actions

// Code for Sift tracking; needs to be manually fired on Android
// See https://github.com/SiftScience/sift-react-native/pull/23#issuecomment-1630984250
const enableAdditionalSiftAndroidTracking = useFeatureFlag(
"AREnableAdditionalSiftAndroidTracking"
)
const trackSiftAndroid = Platform.OS === "android" && enableAdditionalSiftAndroidTracking
const routeNameRef = useRef()
useEffect(() => {
if (trackSiftAndroid) {
const initialRouteName = routeNameRef.current
mdole marked this conversation as resolved.
Show resolved Hide resolved
// Expect error becuase their typing doesn't include setPageName, but it's there
// @ts-expect-error
mdole marked this conversation as resolved.
Show resolved Hide resolved
SiftReactNative.setPageName(`screen_${initialRouteName}`)
SiftReactNative.upload()
}
}, [])
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, even better, you might want to move this to be within onReady prop in the NavigationContainer to avoid watching the onReady global state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thank you! Updated in 3d08ba5


return (
<NavigationContainer
ref={__unsafe_mainModalStackRef}
Expand Down Expand Up @@ -51,6 +72,12 @@ export const ModalStack: React.FC = ({ children }) => {
data: { ...params },
level: Severity.Info,
})

if (trackSiftAndroid) {
// @ts-expect-error
SiftReactNative.setPageName(`screen_${currentRoute.name}`)
SiftReactNative.upload()
}
}
}}
>
Expand Down