Skip to content

Comments

Add SentryFeedbackWidget#2240

Merged
denrase merged 21 commits intofeat/capture-feedbackfrom
feat/feedback-widget
Sep 23, 2024
Merged

Add SentryFeedbackWidget#2240
denrase merged 21 commits intofeat/capture-feedbackfrom
feat/feedback-widget

Conversation

@denrase
Copy link
Collaborator

@denrase denrase commented Aug 20, 2024 •
edited
Loading

📜 Description

  • Adds a generic feedback widget to flutter package
Feedback Widget
feedback

💡 Motivation and Context

Relates to #1593

💚 How did you test it?

  • Widget Tests

📝 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

🔮 Next steps

With sentry-javascrip we can also add a screenshot of the browser window as an attachment. What makes sense here?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 20, 2024 •
edited
Loading

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

Generated by 🚫 dangerJS against 9645324

@codecov
Copy link

codecov bot commented Aug 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.91837% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.92%. Comparing base (55c6870) to head (9645324).
Report is 1 commits behind head on feat/capture-feedback.

Files with missing lines Patch % Lines
...utter/lib/src/feedback/sentry_feedback_widget.dart 95.91% 4 Missing ⚠️
Additional details and impacted files 
@@                    Coverage Diff                    @@
##           feat/capture-feedback    #2240      +/-   ##
=========================================================
+ Coverage                  88.03%   90.92%   +2.88%     
=========================================================
  Files                        248       74     -174     
  Lines                       8653     2468    -6185     
=========================================================
- Hits                        7618     2244    -5374     
+ Misses                      1035      224     -811

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

@denrase denrase changed the title Feedback Widget Add SentryFeedbackWidget Aug 20, 2024
ueman
ueman reviewed Aug 20, 2024
View reviewed changes
@denrase denrase requested a review from ueman August 20, 2024 13:25
@denrase denrase marked this pull request as ready for review August 20, 2024 13:25
ueman
ueman reviewed Aug 20, 2024
View reviewed changes
@ueman
Copy link
Collaborator

ueman commented Aug 26, 2024

With sentry-javascrip we can also add a screenshot of the browser window as an attachment. What makes sense here?

Maybe the screenshot can be taken right before the feedback form is triggered?

It could work something like this:

Future<void> showSentryFeedbackForm(BuildContext context) {
  await takeScreenshot();
  await Navigator.push(
    context,
    MaterialPageRoute(
      builder: (context) => SentryFeedbackWidget(associatedEventId: id),
      fullscreenDialog: true,
    ),
  );
}

This follows the patter as to what Flutter provides out of the box with showDialog(), showBottomSheet() etc.

I doubt it's possible to do while the feedback from is active, since the form would be captured by the screenshot widget too.

@buenaflor
Copy link
Contributor

buenaflor commented Aug 27, 2024

With sentry-javascript we can also add a screenshot of the browser window as an attachment

afaik js browser sdk doesn't support screenshots, don't we support screenshot on web anyway?

@denrase
Merge branch 'feat/capture-feedback' into feat/feedback-widget
2b2d0b2 
# Conflicts:
#	flutter/example/lib/user_feedback_dialog.dart
#	flutter/test/mocks.mocks.dart
@denrase
Copy link
Collaborator Author

denrase commented Sep 23, 2024

@ueman I will merge this PR into the other. Let's tackle auto-screenshots in another PR.

@denrase denrase merged commit f1a8281 into feat/capture-feedback Sep 23, 2024
@denrase denrase deleted the feat/feedback-widget branch September 23, 2024 11:38
buenaflor added a commit that referenced this pull request Oct 10, 2024
@denrase @buenaflor
Support captureFeedback (#2230)
0880a97 
* add captureFeedback methods

* add should capture feedback as event test

* add sentry_feedback_test

* update contexts test

* test before send feedback

* test hint and event processors

* basic scope test

* test trace context and attachment behaviour

* test sample rate for feedback and fix mock transport calls comparison

* add hub tests

* add sentry tests

* add changelog entry

* cleanup + comments

* test envelope item for feedback

* remove duplacte typedef

* fix test expectation

* Deprecate captureUserFeedback

* update depraction info in cl

* format

* add missing option in test

* run format

* organize imports

* add missing method

* fix test epectation

* ignore deprecations internally

* add to integration test, fix analyze errors

* fix cl

* disable fixture.options.automatedTestMode

* update test

* fix cl

* Add `SentryFeedbackWidget` (#2240)

* fix cl

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

---------

Co-authored-by: Giancarlo Buenaflor <[email protected]>
@bruno-garcia bruno-garcia mentioned this pull request Nov 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich

@stefanosiano stefanosiano Awaiting requested review from stefanosiano

@buenaflor buenaflor Awaiting requested review from buenaflor

@martinhaintz martinhaintz Awaiting requested review from martinhaintz

1 more reviewer

@ueman ueman ueman left review comments

Reviewers whose approvals may not affect merge requirements

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 @ueman @buenaflor