-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: add analytics tracking for account pin, hide, and list viewed events #37952
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
base: main
Are you sure you want to change the base?
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
✨ Files requiring CODEOWNER review ✨🔑 @MetaMask/accounts-engineers (2 files, +58 -3)
|
| // Calculate the count after the action | ||
| const pinnedCountAfter = willBePinned | ||
| ? pinnedAccountsList.length + 1 | ||
| : pinnedAccountsList.length - 1; |
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.
Bug: Account Group Pinning: Count Mismatch
The pinnedCountAfter calculation assumes only one address is added or removed, but account groups can contain multiple addresses across different chains. When pinning or unpinning a group with N addresses, the count changes by N, not 1. The calculation uses stale list values captured at render time and doesn't account for the actual number of addresses in the account group being modified.
| // Calculate the count after the action | ||
| const hiddenCountAfter = willBeHidden | ||
| ? hiddenAccountsList.length + 1 | ||
| : hiddenAccountsList.length - 1; |
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.
Bug: Hidden Count Mismatch for Account Groups
The hiddenCountAfter calculation assumes only one address is added or removed, but account groups can contain multiple addresses across different chains. When hiding or unhiding a group with N addresses, the count changes by N, not 1. The calculation uses stale list values captured at render time and doesn't account for the actual number of addresses in the account group being modified.
Builds ready [2db678e]
UI Startup Metrics (1248 ± 93 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
owencraston
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.
The Account Pinned/Hidden events were firing but some of the property values were not correct. I did not see the Account List Viewed event fired at all.
Here is a summary of the events that fired and some of the issues in comments
[mock-segment]: Track event received: Account Pinned
{
"pinned": true,
"pinned_count_after": 1,
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Pinned
{
"pinned": false,
"pinned_count_after": -1, // should have been 0 since I removed the only hidden account
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Hidden
{
"hidden": true,
"hidden_count_after": 1,
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Hidden
{
"hidden": false,
"hidden_count_after": -1, // should have been 0 since I removed the only hidden account
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Pinned
{
"pinned": true,
"pinned_count_after": 1,
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Pinned
{
"pinned": true,
"pinned_count_after": 1, // should have been 2 since I had two hidden accounts
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Hidden
{
"hidden": true,
"hidden_count_after": 1,
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Hidden
{
"hidden": true,
"hidden_count_after": 1, should have been 2 since I hid two accounts
"category": "Accounts",
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
[mock-segment]: Track event received: Account Hidden
{
"hidden": false,
"hidden_count_after": -1, // should have been 1 since there was one hidden account
"locale": "en",
"chain_id": "0xaa36a7",
"environment_type": "popup"
}
Description
Add analytics tracking for the Pin & Hide Accounts feature to understand user engagement with account organization features.
This PR implements three new Segment events:
These events enable analysis of:
Changelog
CHANGELOG entry: null
Related issues
MUL-125
Requires: https://github.com/Consensys/segment-schema/pull/353
Manual testing steps
Account Pinnedevent fires withpinned: trueandpinned_count_afterAccount Pinnedevent fires withpinned: falseand updated countAccount Hiddenevent fires withhidden: trueandhidden_count_afterAccount Hiddenevent fires withhidden: falseAccount List Viewedevent fires withpinned_count,hidden_count, andtotal_accountsScreenshots/Recordings
Before
No analytics tracking for pin/hide account features
After
Analytics
Note
Add analytics for pin/hide actions in the multichain account menu and define new event names, with count properties and related state handling.
MetaMetricsEventNameentries:AccountPinned,AccountHiddeninshared/constants/metametrics.ts.MetaMetricsContextwith propertiespinned/hiddenandpinned_count_after/hidden_count_after.getPinnedAccountsListandgetHiddenAccountsListto compute post-action counts.pinnedAccountListandhiddenAccountList.multichain-account-menu.test.tsx.Written by Cursor Bugbot for commit 2db678e. This will update automatically on new commits. Configure here.