Skip to content

Add screenshot to SentryFeedbackWidget#2369

Merged
denrase merged 11 commits into
mainfrom
feat/add-screenshot-to-feedback-widget
Oct 29, 2024
Merged

Add screenshot to SentryFeedbackWidget#2369
denrase merged 11 commits into
mainfrom
feat/add-screenshot-to-feedback-widget

Conversation

@denrase

@denrase denrase commented Oct 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📜 Description

Add the option to add a screenshot SentryAttachment to the SentryFeedbackWidget

Bildschirmfoto 2024-10-22 um 17 20 47

💡 Motivation and Context

Closes to #1593

💚 How did you test it?

Unit tests, example app.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@github-actions

github-actions Bot commented Oct 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 8d54f66

@codecov

codecov Bot commented Oct 22, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 52.38095% with 10 lines in your changes missing coverage. Please review.

Project coverage is 84.84%. Comparing base (0479083) to head (8d54f66).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
flutter/lib/src/sentry_flutter.dart 0.00% 10 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2369      +/-   ##
==========================================
+ Coverage   84.78%   84.84%   +0.05%     
==========================================
  Files         253       79     -174     
  Lines        9070     2804    -6266     
==========================================
- Hits         7690     2379    -5311     
+ Misses       1380      425     -955

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@denrase denrase changed the title Add screenshot wo feedback widget Add screenshot to SentryFeedbackWidget Oct 22, 2024
@denrase

denrase commented Oct 22, 2024

Copy link
Copy Markdown
Collaborator Author

@buenaflor I'm not sure if moving the code that takes the screenshot to the SentryScreenshotWidget is even neccessary, as the user facing API is SentryFlutter.captureFeedback. Pls add ur 2 cents.

@denrase denrase marked this pull request as ready for review October 22, 2024 15:39
@github-actions

github-actions Bot commented Oct 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 467.14 ms 504.72 ms 37.58 ms
Size 6.49 MiB 7.57 MiB 1.08 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e2d89fc 323.84 ms 376.23 ms 52.39 ms
be173fa 375.94 ms 458.36 ms 82.42 ms
7b2e0ad 415.59 ms 457.26 ms 41.67 ms
322aa66 284.98 ms 341.76 ms 56.78 ms
eecbbca 324.37 ms 352.49 ms 28.12 ms
f3a18f2 456.29 ms 504.41 ms 48.12 ms
e0ba81f 390.38 ms 465.40 ms 75.02 ms
33527b4 403.58 ms 507.44 ms 103.86 ms
3334ac1 303.98 ms 366.65 ms 62.67 ms
5e7abc5 360.82 ms 401.18 ms 40.37 ms

App size

Revision Plain With Sentry Diff
e2d89fc 6.06 MiB 7.03 MiB 989.37 KiB
be173fa 6.35 MiB 7.33 MiB 1005.54 KiB
7b2e0ad 6.52 MiB 7.61 MiB 1.08 MiB
322aa66 5.94 MiB 6.92 MiB 1005.75 KiB
eecbbca 5.94 MiB 6.89 MiB 975.78 KiB
f3a18f2 6.49 MiB 7.57 MiB 1.08 MiB
e0ba81f 6.52 MiB 7.59 MiB 1.06 MiB
33527b4 6.35 MiB 7.42 MiB 1.07 MiB
3334ac1 6.06 MiB 7.03 MiB 993.54 KiB
5e7abc5 6.34 MiB 7.28 MiB 966.66 KiB

Previous results on branch: feat/add-screenshot-to-feedback-widget

Startup times

Revision Plain With Sentry Diff
10ecd8c 461.74 ms 480.69 ms 18.95 ms
be5c603 465.50 ms 505.94 ms 40.44 ms
6a14392 514.13 ms 562.09 ms 47.96 ms

App size

Revision Plain With Sentry Diff
10ecd8c 6.49 MiB 7.57 MiB 1.08 MiB
be5c603 6.49 MiB 7.57 MiB 1.08 MiB
6a14392 6.49 MiB 7.57 MiB 1.08 MiB

@github-actions

github-actions Bot commented Oct 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1216.82 ms 1257.36 ms 40.55 ms
Size 8.38 MiB 9.75 MiB 1.37 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8cb6557 1265.14 ms 1266.08 ms 0.94 ms
bc29768 1247.25 ms 1268.63 ms 21.38 ms
3e5078c 1239.73 ms 1248.69 ms 8.96 ms
6957bfd 1283.80 ms 1289.00 ms 5.20 ms
0ceb89c 1252.02 ms 1271.78 ms 19.75 ms
d883d62 1221.39 ms 1230.18 ms 8.80 ms
1d47eb7 1212.57 ms 1222.00 ms 9.43 ms
21d4150 1252.86 ms 1280.55 ms 27.69 ms
6a5a65d 1237.22 ms 1250.29 ms 13.07 ms
b0811cc 1238.23 ms 1255.82 ms 17.59 ms

App size

Revision Plain With Sentry Diff
8cb6557 8.10 MiB 9.18 MiB 1.08 MiB
bc29768 8.32 MiB 9.38 MiB 1.05 MiB
3e5078c 8.28 MiB 9.34 MiB 1.06 MiB
6957bfd 8.15 MiB 9.15 MiB 1020.71 KiB
0ceb89c 8.15 MiB 9.12 MiB 989.78 KiB
d883d62 8.29 MiB 9.36 MiB 1.07 MiB
1d47eb7 8.29 MiB 9.39 MiB 1.09 MiB
21d4150 8.16 MiB 9.17 MiB 1.01 MiB
6a5a65d 8.34 MiB 9.65 MiB 1.31 MiB
b0811cc 8.32 MiB 9.38 MiB 1.06 MiB

Previous results on branch: feat/add-screenshot-to-feedback-widget

Startup times

Revision Plain With Sentry Diff
6a14392 1250.48 ms 1264.91 ms 14.43 ms
be5c603 1242.40 ms 1265.90 ms 23.50 ms
10ecd8c 1228.69 ms 1242.33 ms 13.63 ms

App size

Revision Plain With Sentry Diff
6a14392 8.38 MiB 9.75 MiB 1.37 MiB
be5c603 8.38 MiB 9.75 MiB 1.37 MiB
10ecd8c 8.38 MiB 9.75 MiB 1.37 MiB

@buenaflor

buenaflor commented Oct 28, 2024

Copy link
Copy Markdown
Contributor

I'm not sure if moving the code that takes the screenshot to the SentryScreenshotWidget is even neccessary, as the user facing API is SentryFlutter.captureFeedback. Pls add ur 2 cents.

Can we keep it in the event processor but still capture the correct screen?

How does the JS user feedback behave in comparison

buenaflor
buenaflor approved these changes Oct 28, 2024
View reviewed changes

@buenaflor buenaflor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@denrase denrase merged commit 7954fb3 into main Oct 29, 2024
@denrase denrase deleted the feat/add-screenshot-to-feedback-widget branch October 29, 2024 13:02
@denrase denrase mentioned this pull request Nov 4, 2024
Closed
@bruno-garcia

bruno-garcia commented Nov 6, 2024

Copy link
Copy Markdown
Member

Let me start with: Thanks! Love this!

Is this on the docs?
Does it support desktop and mobile? (rather: which platforms does it support?

We'd like to tell a holistic story about our feedback product and capabilities, and include all mobile support. @armcknight added support for iOS, and @romtsn had a PR for Android. More SDKs will come.

cc @jas-kas PM of User Feedback

martinhaintz added a commit that referenced this pull request Nov 11, 2024
@martinhaintz
Merge branch 'main' into feat/redact-screenshots-via-view-hierarchy
9699edc 
* main:
  release: 8.10.1
  fix: android build error when compiling (#2397)
  release: 8.10.0
  chore: prepare changelog for `8.10.0` release (#2391)
  chore(deps): update Cocoa SDK to v8.40.1 (#2394)
  fix: cocoa sdk version updater (#2392)
  Send Less Client Reports When Rate Limited (#2380)
  build(deps): bump ruby/setup-ruby from 1.197.0 to 1.199.0 (#2386)
  chore(deps): update Native SDK to v0.7.12 (#2390)
  chore(deps): update Android SDK to v7.16.0 (#2373)
  fix build error for latest flutter beta (3.27.0) (#2385)
  Remove duplicate tests in sentry_client_test.dart (#2378)
  Handle backpressure earlier in pipeline (#2371)
  Add screenshot to `SentryFeedbackWidget` (#2369)

# Conflicts:
#	flutter/lib/src/event_processor/screenshot_event_processor.dart
@bruno-garcia bruno-garcia mentioned this pull request Nov 20, 2024
Closed
@kahest kahest mentioned this pull request Jan 28, 2025
Closed
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@buenaflor buenaflor buenaflor approved these changes

@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich

@stefanosiano stefanosiano Awaiting requested review from stefanosiano

@martinhaintz martinhaintz Awaiting requested review from martinhaintz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@denrase @buenaflor @bruno-garcia