Skip to content

feat(feedback): Have screenshot by default #11839

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 1 commit into from
Apr 30, 2024
Merged

Conversation

c298lee
Copy link
Contributor

@c298lee c298lee commented Apr 29, 2024

Defaults to having screenshots.

@c298lee c298lee requested a review from a team April 29, 2024 20:19
@@ -64,7 +64,7 @@ export const buildFeedbackIntegration = ({
autoInject = true,
showEmail = true,
showName = true,
showScreenshot = false,
showScreenshot = true,
Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify, with the current state of things, this will lazy load the screenshots integration unless it is already installed, both on NPM & CDN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe @ryan953 can confirm, but yes I think so. Is that not something we want to do?

Copy link
Member

@ryan953 ryan953 Apr 30, 2024

Choose a reason for hiding this comment

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

Before #11791 we're shipping the sync strategy using the name feedbackIntegration in all places.

Long term, #11791 sets things up. From that PR:

  • packages/browser/src/index.ts is returning feedbackSyncIntegration as feedbackIntegration, so that should be the Sync strategy for NPM users, by default. But both strats are available.
  • index.bundle.* (there are two) returning feedbackAsyncIntegration as feedbackIntegration, which is the async strategy, sync is not available (we'd need another bundle build to do that)

@c298lee c298lee merged commit ffcdc5c into develop Apr 30, 2024
91 checks passed
@c298lee c298lee deleted the cl/show-screenshot branch April 30, 2024 16:05
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.

3 participants