-
-
Notifications
You must be signed in to change notification settings - Fork 263
SentryFeedbackWidget Improvements #2964
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2964 +/- ##
==========================================
+ Coverage 87.72% 88.03% +0.31%
==========================================
Files 286 287 +1
Lines 9335 9528 +193
==========================================
+ Hits 8189 8388 +199
+ Misses 1146 1140 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8ced2dc | 295.58 ms | 336.49 ms | 40.91 ms |
4b29d6e | 386.80 ms | 430.86 ms | 44.06 ms |
5f443de | 412.30 ms | 491.67 ms | 79.37 ms |
04db237 | 330.16 ms | 428.38 ms | 98.22 ms |
689d2fd | 378.62 ms | 430.48 ms | 51.86 ms |
6572f8d | 302.35 ms | 348.10 ms | 45.75 ms |
ccc09e4 | 308.21 ms | 357.74 ms | 49.54 ms |
7954fb3 | 459.38 ms | 500.72 ms | 41.34 ms |
33f99c9 | 668.33 ms | 735.08 ms | 66.76 ms |
dd5521e | 454.51 ms | 538.29 ms | 83.78 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8ced2dc | 6.06 MiB | 7.03 MiB | 990.29 KiB |
4b29d6e | 6.33 MiB | 7.26 MiB | 946.14 KiB |
5f443de | 6.35 MiB | 7.34 MiB | 1008.00 KiB |
04db237 | 5.94 MiB | 6.95 MiB | 1.01 MiB |
689d2fd | 6.06 MiB | 7.10 MiB | 1.04 MiB |
6572f8d | 6.15 MiB | 7.13 MiB | 999.97 KiB |
ccc09e4 | 5.94 MiB | 6.95 MiB | 1.01 MiB |
7954fb3 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
33f99c9 | 6.54 MiB | 7.53 MiB | 1017.45 KiB |
dd5521e | 6.44 MiB | 7.50 MiB | 1.06 MiB |
Previous results on branch: feat/imporve-user-feedback-widget
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d5bfab8 | 450.79 ms | 492.04 ms | 41.25 ms |
f84f616 | 470.73 ms | 503.86 ms | 33.13 ms |
5848af1 | 426.16 ms | 489.23 ms | 63.07 ms |
d99d55b | 458.37 ms | 524.06 ms | 65.69 ms |
2a151e2 | 492.16 ms | 564.78 ms | 72.62 ms |
23ff8a6 | 493.56 ms | 556.38 ms | 62.82 ms |
62f0687 | 457.10 ms | 518.62 ms | 61.52 ms |
a5ea1aa | 453.20 ms | 520.36 ms | 67.16 ms |
3687d4f | 441.35 ms | 501.16 ms | 59.81 ms |
9592079 | 474.51 ms | 530.07 ms | 55.55 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d5bfab8 | 6.54 MiB | 7.53 MiB | 1016.51 KiB |
f84f616 | 6.54 MiB | 7.53 MiB | 1016.51 KiB |
5848af1 | 6.54 MiB | 7.53 MiB | 1017.67 KiB |
d99d55b | 6.54 MiB | 7.53 MiB | 1017.81 KiB |
2a151e2 | 6.54 MiB | 7.53 MiB | 1017.18 KiB |
23ff8a6 | 6.44 MiB | 7.50 MiB | 1.05 MiB |
62f0687 | 6.44 MiB | 7.50 MiB | 1.05 MiB |
a5ea1aa | 6.54 MiB | 7.53 MiB | 1016.51 KiB |
3687d4f | 6.54 MiB | 7.53 MiB | 1017.67 KiB |
9592079 | 6.44 MiB | 7.51 MiB | 1.06 MiB |
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c1bb00f | 1265.14 ms | 1290.85 ms | 25.71 ms |
f172c4d | 1350.66 ms | 1408.49 ms | 57.83 ms |
0be962b | 1264.10 ms | 1281.16 ms | 17.06 ms |
6325c3b | 1266.52 ms | 1291.06 ms | 24.54 ms |
e1897c5 | 1252.61 ms | 1276.48 ms | 23.87 ms |
aa950e9 | 1275.17 ms | 1295.33 ms | 20.16 ms |
a49594a | 1284.83 ms | 1313.29 ms | 28.45 ms |
1131914 | 1277.20 ms | 1300.20 ms | 23.00 ms |
7659cbe | 1246.70 ms | 1265.88 ms | 19.17 ms |
3f23617 | 1261.93 ms | 1286.10 ms | 24.17 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c1bb00f | 8.09 MiB | 9.07 MiB | 1001.06 KiB |
f172c4d | 8.33 MiB | 9.62 MiB | 1.29 MiB |
0be962b | 8.10 MiB | 9.16 MiB | 1.07 MiB |
6325c3b | 8.16 MiB | 9.17 MiB | 1.01 MiB |
e1897c5 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
aa950e9 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
a49594a | 8.16 MiB | 9.16 MiB | 1.00 MiB |
1131914 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
7659cbe | 8.42 MiB | 9.89 MiB | 1.47 MiB |
3f23617 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
Previous results on branch: feat/imporve-user-feedback-widget
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3687d4f | 1264.86 ms | 1281.39 ms | 16.53 ms |
09c60a5 | 1247.39 ms | 1270.30 ms | 22.91 ms |
2a151e2 | 1243.96 ms | 1255.74 ms | 11.79 ms |
62f0687 | 1244.63 ms | 1263.07 ms | 18.44 ms |
23ff8a6 | 1241.63 ms | 1260.20 ms | 18.57 ms |
5848af1 | 1242.07 ms | 1258.90 ms | 16.83 ms |
d99d55b | 1266.29 ms | 1274.09 ms | 7.79 ms |
a5ea1aa | 1234.21 ms | 1243.83 ms | 9.63 ms |
f84f616 | 1254.57 ms | 1281.06 ms | 26.49 ms |
d5bfab8 | 1264.50 ms | 1277.04 ms | 12.54 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3687d4f | 7.85 MiB | 9.42 MiB | 1.57 MiB |
09c60a5 | 8.43 MiB | 10.05 MiB | 1.62 MiB |
2a151e2 | 7.85 MiB | 9.45 MiB | 1.59 MiB |
62f0687 | 8.43 MiB | 10.04 MiB | 1.61 MiB |
23ff8a6 | 8.43 MiB | 10.04 MiB | 1.61 MiB |
5848af1 | 7.85 MiB | 9.42 MiB | 1.57 MiB |
d99d55b | 7.85 MiB | 9.45 MiB | 1.59 MiB |
a5ea1aa | 7.85 MiB | 9.45 MiB | 1.59 MiB |
f84f616 | 7.85 MiB | 9.45 MiB | 1.59 MiB |
d5bfab8 | 7.85 MiB | 9.45 MiB | 1.59 MiB |
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.
I just tested it and did the following
- Opened feedback
- Add my name
- Add my email
- Add text
- Tap Capture Screenshot button
- Feedback screen hides
- Take Screenshot
- Feedback screen shows
-> my text input (name, email, body) is gone
So we should make it so the input stays after capturing a screenshot
@buenaflor Ok, then we need to implement some kind of (memory) persistence. |
@denrase Did you look into the |
@ueman Will check it out, thx for the tip. Just static state within the widget that we then clear was also the first thing that came to my mind. |
@ueman Checked the docs for SR. It is described that unique ID on iOS on the project level has to be set in the |
I haven't used the API yet, but the docs indeed imply that the user needs to tke action which is very unfortunate |
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.
We want to track usage of the Widget (dev usage, not usage of the user who opens the widget) so we should add something like options.sdk.integrations('MobileFeedbackWidget')
;
Maybe we find a way to add it even if the user hasn't opened the widget yet - if at all possible? not sure
@buenaflor Added the integration when the options are accessed. This is not 100% accurate if we want to track if developers are using the feature, but it's the closest we can get with the current structure. |
|
||
/// Options for the [SentryFeedbackWidget] | ||
SentryFeedbackOptions get feedback { | ||
sdk.addIntegration('MobileFeedbackWidget'); |
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.
won't this keep adding the integration if you access the feedback multiple times
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.
Thought so too, but the addIntegration method already checks for this:
// Adds an integration if not already added
void addIntegration(String integration) {
if (_integrations.contains(integration)) {
return;
}
_integrations.add(integration);
}
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.
okay cool
also let's add a small comment why we add this here.
looks confusing without context
📜 Description
pick screenshot💡 Motivation and Context
Closes #2697
💚 How did you test it?
📝 Checklist
sendDefaultPii
is enabled🔮 Next steps
Document iOS/Android strings from camera roll accessnot needed with current setupDouble check Android issue with app being terminated while picking image.