-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Recommend App: Add Tracks #17027
Recommend App: Add Tracks #17027
Conversation
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
You can trigger an installable build for these changes by visiting CircleCI 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.
Howdy @dvdchr 👋
Tested the events and confirmed they were dispatched as expected. For the failure event I confirmed by disabling wifi. Looks good!
Generated by 🚫 dangerJS |
Thank you! |
Refs #16995,
pctCYC-8X-p2#comment-340
Depends on #17020
Summary
This adds tracking events in the
ShareAppContentPresenter
, with details as described inpctCYC-8X-p2#comment-340
. Additionally, I updatedTestAnalyticsTracker
to storetrackString
calls (for unit testing purposes).Note: I'll keep this PR in open until the middle of next week, in case if there are any feedback on the event names, etc.
To Test
There's no visual change, but tests for tracking verification is added. Make sure the tests are ✅ !
Regression Notes
Potential unintended areas of impact
None.
What I did to test those areas of impact (or what existing automated tests I relied on)
Added unit tests for the tracking implementation.
What automated tests I added (or what prevented me from doing so)
Added unit tests for the tracking implementation.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.