-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(feedback): New feedback integration with screenshots #10590
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
Changes from all commits
Commits
Show all changes
62 commits
Select commit
Hold shift + click to select a range
7a22a83
working feedback screenshot package with preact
c298lee 4877b4f
working feedback screenshot package with preact
c298lee ef7d7e8
update rollup
c298lee 0395331
allow passing options to sucrase + pass jsxPragma
billyvg 364e187
proper ts-expect-error
billyvg 510f37c
fix for preact
billyvg 513ab3e
change to tsx and fix for preact
billyvg 7d9b800
screenshot button
c298lee 8dbd500
screenshot button and widget placeholders
c298lee 4a007a1
Make feedback depend on feedback-screenshot for now, until we have ay…
ryan953 0de030f
Convert screenshotButton to a functional component, and add types to …
ryan953 8ea52d3
Update docblock for feedbackScreenshotIntegration
ryan953 d30f236
take screenshot
c298lee f9b6f31
take screenshot
c298lee 354bb52
move screenshot into form
c298lee 22efe6e
working screenshot attachment
c298lee 22dad05
Merge branch 'develop' into cl/screenshot-integration
c298lee ea7b909
clean up attachment code
c298lee 1b216b9
remove alias plugin
c298lee b03f61a
remove dialog when screenshotting
c298lee 30e9c40
wip: cropping box follows mouse, need to remove screenshot editor & h…
c298lee a1771bd
cropping works for 1 cornergit add .
c298lee ea96f76
Ryan953/feedback async (#10683)
ryan953 4447057
4 corners working! need to fix resetting image
c298lee 5122a60
fix cropping & button styling
c298lee a38d9c5
disable screenshots in mobile browsers
ryan953 752039a
Merge branch 'develop' into cl/screenshot-integration
c298lee 1c94e75
update import path
ryan953 4312d30
bump versions of new packages to match latest release
ryan953 c4124b7
Merge branch 'develop' into cl/screenshot-integration
c298lee e19a876
cropping works with feedback 2! need to clean up & refactor
c298lee 2649607
clean up
c298lee 7d39df4
revert feedback form
c298lee e148ef5
Merge branch 'develop' into cl/screenshot-integration
c298lee 8a08bbd
some styling
c298lee 359a379
rename to imageBuffer
ryan953 cfdb29e
add return types to all the callbacks
ryan953 61c9a5a
wip - managing canvas drawing
ryan953 3a60a19
bringing cropping handles inside the modal
ryan953 1bcd4ae
follow rule of hooks in Form, and render the gray background always
ryan953 f6355cb
start the selection size as large as possible, reset it whenever wind…
ryan953 2786d6a
bump versions of new packages to match latest release
ryan953 824c7b6
everything functioning including resizing & tall images :D
c298lee b0d2ad5
remove feedback-screenshot
c298lee 1f89d8a
Merge branch 'develop' into cl/screenshot-integration
c298lee ce0be69
don't allow crop stuff to go outside of the image
c298lee d8b6242
Merge branch 'develop' into cl/screenshot-integration
c298lee c106fd6
rename feedback2 to feedback
ryan953 c73ab4b
crop buttons don't move past the other crop buttons
c298lee 84ed105
disable lint & move styling
c298lee 40cc76a
ignore file check and fix name
c298lee 24b3956
remove extra eslint rules, re-arrange package.json to keep diff tidy
ryan953 50fad97
rm extra css comment, and rm unneeded line-move
ryan953 75a37f8
fix indent
ryan953 fc8b8c7
fix indent
ryan953 d6f70fe
Merge branch 'develop' into cl/screenshot-integration
ryan953 1621e61
convert indent to spaces
ryan953 b5ed596
add specific error message when response.statusCode === 0
ryan953 b33d2df
keep the original test for sendFeedback()
ryan953 b2016cf
drop preact-compat, we use sucrase and set `jsxPragma: h,`
ryan953 c0ec628
Merge branch 'develop' into cl/screenshot-integration
c298lee 4591603
Try aliasing preact typescript declaration to fix ts errors (#11088)
AbhiPrasad File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,15 @@ | ||
import { makeBaseBundleConfig, makeBundleConfigVariants } from '@sentry-internal/rollup-utils'; | ||
|
||
const baseBundleConfig = makeBaseBundleConfig({ | ||
bundleType: 'addon', | ||
entrypoints: ['src/index.ts'], | ||
licenseTitle: '@sentry-internal/feedback', | ||
outputFileBase: () => 'bundles/feedback', | ||
}); | ||
|
||
const builds = makeBundleConfigVariants(baseBundleConfig); | ||
|
||
export default builds; | ||
export default makeBundleConfigVariants( | ||
makeBaseBundleConfig({ | ||
bundleType: 'addon', | ||
entrypoints: ['src/index.ts'], | ||
jsVersion: 'es6', | ||
licenseTitle: '@sentry-internal/feedback', | ||
outputFileBase: () => 'bundles/feedback', | ||
sucrase: { | ||
jsxPragma: 'h', | ||
jsxFragmentPragma: 'Fragment', | ||
}, | ||
}), | ||
); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
// preact does not support more modern TypeScript versions, which breaks our users that depend on older | ||
// TypeScript versions. To fix this, we shim the types from preact to be any and remove the dependency on preact | ||
// for types directly. This script is meant to be run after the build/npm/types-ts3.8 directory is created. | ||
|
||
// Path: build/npm/types-ts3.8/global.d.ts | ||
|
||
const fs = require('fs'); | ||
const path = require('path'); | ||
|
||
/** | ||
* This regex looks for preact imports we can replace and shim out. | ||
* | ||
* Example: | ||
* import { ComponentChildren, VNode } from 'preact'; | ||
*/ | ||
const preactImportRegex = /import\s*{\s*([\w\s,]+)\s*}\s*from\s*'preact'\s*;?/; | ||
|
||
function walk(dir) { | ||
const files = fs.readdirSync(dir); | ||
files.forEach(file => { | ||
const filePath = path.join(dir, file); | ||
const stat = fs.lstatSync(filePath); | ||
if (stat.isDirectory()) { | ||
walk(filePath); | ||
} else { | ||
if (filePath.endsWith('.d.ts')) { | ||
const content = fs.readFileSync(filePath, 'utf8'); | ||
Check failureCode scanning / CodeQL Potential file system race condition
The file may have changed since it [was checked](1).
|
||
const capture = preactImportRegex.exec(content); | ||
if (capture) { | ||
const groups = capture[1].split(',').map(s => s.trim()); | ||
|
||
// This generates a shim snippet to replace the type imports from preact | ||
// It generates a snippet based on the capture groups of preactImportRegex. | ||
// | ||
// Example: | ||
// | ||
// import type { ComponentChildren, VNode } from 'preact'; | ||
// becomes | ||
// type ComponentChildren: any; | ||
// type VNode: any; | ||
const snippet = groups.reduce((acc, curr) => { | ||
const searchableValue = curr.includes(' as ') ? curr.split(' as ')[1] : curr; | ||
|
||
// look to see if imported as value, then we have to use declare const | ||
if (content.includes(`typeof ${searchableValue}`)) { | ||
return `${acc}declare const ${searchableValue}: any;\n`; | ||
} | ||
|
||
// look to see if generic type like Foo<T> | ||
if (content.includes(`${searchableValue}<`)) { | ||
return `${acc}type ${searchableValue}<T> = any;\n`; | ||
} | ||
|
||
// otherwise we can just leave as type | ||
return `${acc}type ${searchableValue} = any;\n`; | ||
}, ''); | ||
|
||
// we then can remove the import from preact | ||
const newContent = content.replace(preactImportRegex, '// replaced import from preact'); | ||
|
||
// and write the new content to the file | ||
fs.writeFileSync(filePath, snippet + newContent, 'utf8'); | ||
Check failureCode scanning / CodeQL Potential file system race condition
The file may have changed since it [was checked](1).
|
||
} | ||
} | ||
} | ||
}); | ||
} | ||
|
||
function run() { | ||
// recurse through build/npm/types-ts3.8 directory | ||
const dir = path.join('build', 'npm', 'types-ts3.8'); | ||
walk(dir); | ||
} | ||
|
||
run(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { GLOBAL_OBJ } from '@sentry/utils'; | ||
|
||
export { DEFAULT_THEME } from './theme'; | ||
|
||
// exporting a separate copy of `WINDOW` rather than exporting the one from `@sentry/browser` | ||
// prevents the browser package from being bundled in the CDN bundle, and avoids a | ||
// circular dependency between the browser and feedback packages | ||
export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; | ||
export const DOCUMENT = WINDOW.document; | ||
export const NAVIGATOR = WINDOW.navigator; | ||
|
||
export const ACTOR_LABEL = 'Report a Bug'; | ||
export const CANCEL_BUTTON_LABEL = 'Cancel'; | ||
export const SUBMIT_BUTTON_LABEL = 'Send Bug Report'; | ||
export const FORM_TITLE = 'Report a Bug'; | ||
export const EMAIL_PLACEHOLDER = '[email protected]'; | ||
export const EMAIL_LABEL = 'Email'; | ||
export const MESSAGE_PLACEHOLDER = "What's the bug? What did you expect?"; | ||
export const MESSAGE_LABEL = 'Description'; | ||
export const NAME_PLACEHOLDER = 'Your Name'; | ||
export const NAME_LABEL = 'Name'; | ||
export const SUCCESS_MESSAGE_TEXT = 'Thank you for your report!'; | ||
|
||
export const FEEDBACK_WIDGET_SOURCE = 'widget'; | ||
export const FEEDBACK_API_SOURCE = 'api'; | ||
|
||
export const SUCCESS_MESSAGE_TIMEOUT = 5000; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,8 @@ | ||
import { GLOBAL_OBJ } from '@sentry/utils'; | ||
|
||
// exporting a separate copy of `WINDOW` rather than exporting the one from `@sentry/browser` | ||
// prevents the browser package from being bundled in the CDN bundle, and avoids a | ||
// circular dependency between the browser and feedback packages | ||
export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; | ||
|
||
const LIGHT_BACKGROUND = '#ffffff'; | ||
const INHERIT = 'inherit'; | ||
const SUBMIT_COLOR = 'rgba(108, 95, 199, 1)'; | ||
const LIGHT_THEME = { | ||
|
||
export const LIGHT_THEME = { | ||
fontFamily: "system-ui, 'Helvetica Neue', Arial, sans-serif", | ||
fontSize: '14px', | ||
|
||
|
@@ -59,18 +53,3 @@ export const DEFAULT_THEME = { | |
error: '#f55459', | ||
}, | ||
}; | ||
|
||
export const ACTOR_LABEL = 'Report a Bug'; | ||
export const CANCEL_BUTTON_LABEL = 'Cancel'; | ||
export const SUBMIT_BUTTON_LABEL = 'Send Bug Report'; | ||
export const FORM_TITLE = 'Report a Bug'; | ||
export const EMAIL_PLACEHOLDER = '[email protected]'; | ||
export const EMAIL_LABEL = 'Email'; | ||
export const MESSAGE_PLACEHOLDER = "What's the bug? What did you expect?"; | ||
export const MESSAGE_LABEL = 'Description'; | ||
export const NAME_PLACEHOLDER = 'Your Name'; | ||
export const NAME_LABEL = 'Name'; | ||
export const SUCCESS_MESSAGE_TEXT = 'Thank you for your report!'; | ||
|
||
export const FEEDBACK_WIDGET_SOURCE = 'widget'; | ||
export const FEEDBACK_API_SOURCE = 'api'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 12 additions & 2 deletions
14
packages/feedback/src/widget/Actor.css.ts → ...feedback/src/core/components/Actor.css.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
we need to send the attachment in a seperate envelope
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.
hmm, this is a bit weird IMHO. We are not really creating an attachment envelope, aren't we? We are just sending a regular (?) event and adding attachments to it...? Or am I misunderstanding this? What is
event
in this case?Could we not just use
client.sendEvent()
here - that takes ahint
which will add any attachments from it. So we could do e.g.:This would also take care of all the metadata etc.
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.
afaik there's something in the backend that's preventing this from working properly, because the old user-feedbacks had their own pipeline/queue or something.
@andrewshie-sentry is working on creating a new queue that'll fix the issue. maybe @JoshFerge can also explain in the meantime. It's really causing a bunch of extra code in here to manage the attachment sending :(
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.
yeah, but we can still send it twice? we are already doing this, should have the same outcome I believe?
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.
yeah, we need to separate out some things in relay / our own consumer long term for this to work well.