-
Notifications
You must be signed in to change notification settings - Fork 226
Analytics | Add support for SuperProperties #2953
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
|
Since we do not need a Session to access this data, I think it's better to put this code in |
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2953 +/- ##
===========================================
+ Coverage 75.56% 75.58% +0.02%
===========================================
Files 1576 1576
Lines 37331 37345 +14
Branches 7253 7260 +7
===========================================
+ Hits 28209 28228 +19
+ Misses 5326 5321 -5
Partials 3796 3796 ☔ View full report in Codecov by Sentry. |
bc7bbd2
to
c3ba703
Compare
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. We could also have added it as some kind of Initializer
in ElementXApplication
since it doesn't have much to do with navigation or UI, but it's not a big deal.
c3ba703
to
37d85c7
Compare
|
Updated
matrix-analytics-events
to 0.23.0 and report SuperProperties for EXA, + test.In order to set the SuperProperties I used a
LaunchedEffect
inLoggedInPresenter
, let me know if fine (Note: this is not the case anymore now in RootPresenter following review)Fixes https://github.com/element-hq/crypto-internal/issues/321
Type of change
Content
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist