-
Notifications
You must be signed in to change notification settings - Fork 593
chore: add auth screen tracking #trivial #13069
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
Conversation
|
|
||
| useEffect(() => { | ||
| tracking.authImpression() | ||
| tracking.authModalScreenView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapping with cohesion provider oddly seemed to affect performance, since we already have this just added it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I see cohesion PR has just been release, let's update cohesion version and use the ownerType.authModal
MounirDhahri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, apart from a few minor comments
| </Flex> | ||
| </MotiView> | ||
| </Box> | ||
| // </KeyboardAvoidingView> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a commented out component i removed
src/app/Scenes/Onboarding/Screens/Auth/hooks/useOnboardingAuthTracking.tsx
Outdated
Show resolved
Hide resolved
a735a8f
e9d99a5 to
55c14e0
Compare
* add authModal screen tracking - untested * useEffect approach to avoid render issues * use string from cohesion * update cohesion to fix types
This PR resolves DIAM-213
Description
Adds an auth screen event to fix issues with unreliable auth analytics.
Should merge this first and use the value from cohesion:
artsy/cohesion#666 😈
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.