Skip to content

feat(feedback): Screenshot button #4714

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

Merged
merged 196 commits into from
Apr 14, 2025
Merged

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Mar 31, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

Based on feat(feedback): Theming

PR Chain:

📜 Description

  • Adds take screenshot button in the feedback widget that:
    • closes the feedback modal
    • shows the screenshot button
  • Adds screenshot button that captures a screenshot and opens the feedback widget modal with the captured screenshot as attachment

⚠️ Note that the screenshot button is not supported on the web because the captureScreenshot() function is only available on mobile. Support for web environment will be investigated on a separate PR.

Simulator.Screen.Recording.-.iPhone.16e.-.2025-04-03.at.12.52.20.mp4

Feedback widget UI states

enableScreenshot (or imagePicker set) enableTakeScreenshot
Simulator Screenshot - iPhone 16e - 2025-04-07 at 18 17 59 Simulator Screenshot - iPhone 16e - 2025-04-07 at 18 20 05
Both Screenshot attached
Simulator Screenshot - iPhone 16e - 2025-04-07 at 18 17 20 Simulator Screenshot - iPhone 16e - 2025-04-07 at 18 17 28

💡 Motivation and Context

Part of #4302

💚 How did you test it?

Sample app

📝 Checklist

  • 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.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

@antonis antonis requested a review from krystofwoldrich April 8, 2025 09:27
@@ -8,20 +8,23 @@ import { defaultScreenshotButtonConfiguration } from './defaults';
import { defaultScreenshotButtonStyles } from './FeedbackWidget.styles';
import { getTheme } from './FeedbackWidget.theme';
import type { ScreenshotButtonProps, ScreenshotButtonStyles, ScreenshotButtonTextConfiguration } from './FeedbackWidget.types';
import { hideScreenshotButton, showFeedbackWidget, showScreenshotButton } from './FeedbackWidgetManager';
Copy link
Collaborator Author

@antonis antonis Apr 14, 2025

Choose a reason for hiding this comment

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

The cyclic dep failure is fixed in #4718

Base automatically changed from antonis/feedback-theme to feedback-ui-2 April 14, 2025 12:29
# Conflicts:
#	CHANGELOG.md
#	packages/core/src/js/feedback/FeedbackWidget.styles.ts
#	packages/core/src/js/feedback/FeedbackWidgetManager.tsx
#	packages/core/src/js/feedback/integration.ts
#	packages/core/test/feedback/FeedbackWidgetManager.test.tsx
@antonis antonis requested a review from krystofwoldrich April 14, 2025 13:00
@@ -180,6 +180,11 @@ describe('ScreenshotButton', () => {
const takeScreenshotButtonAfterCapture = queryByText('Take a screenshot');
expect(takeScreenshotButtonAfterCapture).toBeNull();
});

await waitFor(() => {
const takeScreenshotButtonAfterCapture = queryByText('Remove screenshot');
Copy link
Member

Choose a reason for hiding this comment

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

L: Just fixed the name and double negation not null.

Suggested change
const takeScreenshotButtonAfterCapture = queryByText('Remove screenshot');
const removeScreenshotButtonAfterCapture = queryByText('Remove screenshot');
expect(removeScreenshotButtonAfterCapture).toBeTruthy();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch Krystof 🙇
Updated with 59597a4

Copy link
Member

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

Looks good, left one more small comment.

Thank you.

@antonis antonis merged commit 4c988a8 into feedback-ui-2 Apr 14, 2025
59 of 60 checks passed
@antonis antonis deleted the antonis/screenshot-button branch April 14, 2025 13:24
antonis added a commit to getsentry/sentry-docs that referenced this pull request Jun 5, 2025
<!-- Use this checklist to make sure your PR is ready for merge. You may
delete any sections you don't need. -->

⚠️ PR Chain:
- #13512
- #13513
- 👉  #13515

## DESCRIBE YOUR PR
*Tell us what you're changing and why. If your PR **resolves an issue**,
please link it so it closes automatically.*

Adds screenshot button documentation.


Part of getsentry/sentry-react-native#4302

Implementation PR:
getsentry/sentry-react-native#4714

## IS YOUR CHANGE URGENT?  

Help us prioritize incoming PRs by letting us know when the change needs
to go live.
- [ ] Urgent deadline (GA date, etc.): <!-- ENTER DATE HERE -->
- [ ] Other deadline: <!-- ENTER DATE HERE -->
- [x] None: Not urgent, can wait up to 1 week+

Should be merged after
getsentry/sentry-react-native#4726 is released

## SLA

- Teamwork makes the dream work, so please add a reviewer to your PRs.
- Please give the docs team up to 1 week to review your PR unless you've
added an urgent due date to it.
Thanks in advance for your help!

## PRE-MERGE CHECKLIST

*Make sure you've checked the following before merging your changes:*

- [ ] Checked Vercel preview for correctness, including links
- [ ] PR was reviewed and approved by any necessary SMEs (subject matter
experts)
- [ ] PR was reviewed and approved by a member of the [Sentry docs
team](https://github.com/orgs/getsentry/teams/docs)

## LEGAL BOILERPLATE

<!-- Sentry employees and contractors can delete or ignore this section.
-->

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.

## EXTRA RESOURCES

- [Sentry Docs contributor guide](https://docs.sentry.io/contributing/)

---------

Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
antonpirker pushed a commit to getsentry/sentry-docs that referenced this pull request Jun 6, 2025
<!-- Use this checklist to make sure your PR is ready for merge. You may
delete any sections you don't need. -->

⚠️ PR Chain:
- #13512
- #13513
- 👉  #13515

## DESCRIBE YOUR PR
*Tell us what you're changing and why. If your PR **resolves an issue**,
please link it so it closes automatically.*

Adds screenshot button documentation.


Part of getsentry/sentry-react-native#4302

Implementation PR:
getsentry/sentry-react-native#4714

## IS YOUR CHANGE URGENT?  

Help us prioritize incoming PRs by letting us know when the change needs
to go live.
- [ ] Urgent deadline (GA date, etc.): <!-- ENTER DATE HERE -->
- [ ] Other deadline: <!-- ENTER DATE HERE -->
- [x] None: Not urgent, can wait up to 1 week+

Should be merged after
getsentry/sentry-react-native#4726 is released

## SLA

- Teamwork makes the dream work, so please add a reviewer to your PRs.
- Please give the docs team up to 1 week to review your PR unless you've
added an urgent due date to it.
Thanks in advance for your help!

## PRE-MERGE CHECKLIST

*Make sure you've checked the following before merging your changes:*

- [ ] Checked Vercel preview for correctness, including links
- [ ] PR was reviewed and approved by any necessary SMEs (subject matter
experts)
- [ ] PR was reviewed and approved by a member of the [Sentry docs
team](https://github.com/orgs/getsentry/teams/docs)

## LEGAL BOILERPLATE

<!-- Sentry employees and contractors can delete or ignore this section.
-->

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.

## EXTRA RESOURCES

- [Sentry Docs contributor guide](https://docs.sentry.io/contributing/)

---------

Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants