-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
Conversation
Co-authored-by: Krystof Woldrich <[email protected]>
@@ -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'; |
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.
The cyclic dep failure is fixed in #4718
# 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
@@ -180,6 +180,11 @@ describe('ScreenshotButton', () => { | |||
const takeScreenshotButtonAfterCapture = queryByText('Take a screenshot'); | |||
expect(takeScreenshotButtonAfterCapture).toBeNull(); | |||
}); | |||
|
|||
await waitFor(() => { | |||
const takeScreenshotButtonAfterCapture = queryByText('Remove screenshot'); |
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.
L: Just fixed the name and double negation not null.
const takeScreenshotButtonAfterCapture = queryByText('Remove screenshot'); | |
const removeScreenshotButtonAfterCapture = queryByText('Remove screenshot'); | |
expect(removeScreenshotButtonAfterCapture).toBeTruthy(); |
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.
Good catch Krystof 🙇
Updated with 59597a4
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.
Looks good, left one more small comment.
Thank you.
📢 Type of change
Based on feat(feedback): Theming
PR Chain:
📜 Description
take screenshot button
in the feedback widget that:screenshot button
screenshot button
that captures a screenshot and opens the feedback widget modal with the captured screenshot as attachmentcaptureScreenshot()
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
(orimagePicker
set)enableTakeScreenshot
💡 Motivation and Context
Part of #4302
💚 How did you test it?
Sample app
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps