-
-
Notifications
You must be signed in to change notification settings - Fork 342
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
(2.2) feat: Add Feedback Form UI Branding logo #4357
(2.2) feat: Add Feedback Form UI Branding logo #4357
Conversation
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a06f6ba+dirty | 381.50 ms | 429.77 ms | 48.27 ms |
1dd8d17+dirty | 383.20 ms | 432.62 ms | 49.41 ms |
d33790a+dirty | 404.87 ms | 473.06 ms | 68.19 ms |
a3ba405+dirty | 359.67 ms | 436.86 ms | 77.19 ms |
0781f75+dirty | 406.72 ms | 454.80 ms | 48.08 ms |
f4a5053+dirty | 391.02 ms | 427.04 ms | 36.02 ms |
03c9048+dirty | 397.35 ms | 417.73 ms | 20.37 ms |
50c70c0+dirty | 385.30 ms | 433.06 ms | 47.76 ms |
cadf235+dirty | 455.51 ms | 451.64 ms | -3.87 ms |
27e1bf3+dirty | 398.69 ms | 439.39 ms | 40.69 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a06f6ba+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
1dd8d17+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
d33790a+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
a3ba405+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
0781f75+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
f4a5053+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
03c9048+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
50c70c0+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
cadf235+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
27e1bf3+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
aee9036+dirty | 378.38 ms | 422.12 ms | 43.74 ms |
d4f7e6b+dirty | 404.04 ms | 454.62 ms | 50.58 ms |
e84c6bb+dirty | 413.28 ms | 470.23 ms | 56.96 ms |
37635cb+dirty | 388.20 ms | 425.66 ms | 37.46 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
aee9036+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
d4f7e6b+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
e84c6bb+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
37635cb+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 1228.06 ms | 1224.43 ms | -3.64 ms |
e0624b6+dirty | 1221.86 ms | 1226.42 ms | 4.57 ms |
d33790a+dirty | 1234.19 ms | 1231.76 ms | -2.43 ms |
a06f6ba+dirty | 1230.45 ms | 1227.09 ms | -3.36 ms |
27e1bf3+dirty | 1230.92 ms | 1232.33 ms | 1.41 ms |
f4a5053+dirty | 1225.32 ms | 1231.47 ms | 6.15 ms |
03c9048+dirty | 1235.37 ms | 1238.15 ms | 2.77 ms |
561640f+dirty | 1220.45 ms | 1227.02 ms | 6.57 ms |
26fc306+dirty | 1227.25 ms | 1225.85 ms | -1.40 ms |
1dd8d17+dirty | 1235.22 ms | 1218.96 ms | -16.27 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 2.36 MiB | 3.11 MiB | 760.92 KiB |
e0624b6+dirty | 2.36 MiB | 3.11 MiB | 761.16 KiB |
d33790a+dirty | 2.36 MiB | 3.11 MiB | 761.06 KiB |
a06f6ba+dirty | 2.36 MiB | 3.11 MiB | 761.35 KiB |
27e1bf3+dirty | 2.36 MiB | 3.11 MiB | 761.03 KiB |
f4a5053+dirty | 2.36 MiB | 3.11 MiB | 761.72 KiB |
03c9048+dirty | 2.36 MiB | 3.11 MiB | 761.74 KiB |
561640f+dirty | 2.36 MiB | 3.11 MiB | 761.19 KiB |
26fc306+dirty | 2.36 MiB | 3.11 MiB | 761.18 KiB |
1dd8d17+dirty | 2.36 MiB | 3.11 MiB | 761.66 KiB |
Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e84c6bb+dirty | 1228.55 ms | 1231.15 ms | 2.60 ms |
d4f7e6b+dirty | 1243.76 ms | 1249.80 ms | 6.04 ms |
aee9036+dirty | 1234.22 ms | 1227.33 ms | -6.90 ms |
37635cb+dirty | 1225.62 ms | 1240.90 ms | 15.28 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e84c6bb+dirty | 2.36 MiB | 3.11 MiB | 765.95 KiB |
d4f7e6b+dirty | 2.36 MiB | 3.11 MiB | 761.07 KiB |
aee9036+dirty | 2.36 MiB | 3.11 MiB | 765.40 KiB |
37635cb+dirty | 2.36 MiB | 3.11 MiB | 761.25 KiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 1226.61 ms | 1225.02 ms | -1.59 ms |
e0624b6+dirty | 1229.19 ms | 1232.18 ms | 3.00 ms |
d33790a+dirty | 1247.14 ms | 1242.86 ms | -4.28 ms |
a06f6ba+dirty | 1235.31 ms | 1238.76 ms | 3.45 ms |
27e1bf3+dirty | 1245.78 ms | 1244.38 ms | -1.40 ms |
f4a5053+dirty | 1233.04 ms | 1240.02 ms | 6.98 ms |
03c9048+dirty | 1231.52 ms | 1225.96 ms | -5.56 ms |
561640f+dirty | 1237.10 ms | 1229.59 ms | -7.51 ms |
26fc306+dirty | 1229.10 ms | 1227.88 ms | -1.22 ms |
1dd8d17+dirty | 1229.28 ms | 1224.92 ms | -4.36 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 2.92 MiB | 3.67 MiB | 773.48 KiB |
e0624b6+dirty | 2.92 MiB | 3.67 MiB | 773.62 KiB |
d33790a+dirty | 2.92 MiB | 3.67 MiB | 773.59 KiB |
a06f6ba+dirty | 2.92 MiB | 3.67 MiB | 773.87 KiB |
27e1bf3+dirty | 2.92 MiB | 3.67 MiB | 773.54 KiB |
f4a5053+dirty | 2.92 MiB | 3.67 MiB | 774.18 KiB |
03c9048+dirty | 2.92 MiB | 3.67 MiB | 774.29 KiB |
561640f+dirty | 2.92 MiB | 3.67 MiB | 773.72 KiB |
26fc306+dirty | 2.92 MiB | 3.67 MiB | 773.77 KiB |
1dd8d17+dirty | 2.92 MiB | 3.67 MiB | 774.21 KiB |
Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e84c6bb+dirty | 1232.96 ms | 1234.90 ms | 1.94 ms |
d4f7e6b+dirty | 1240.88 ms | 1236.92 ms | -3.96 ms |
aee9036+dirty | 1242.10 ms | 1244.76 ms | 2.65 ms |
37635cb+dirty | 1233.54 ms | 1229.82 ms | -3.73 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e84c6bb+dirty | 2.92 MiB | 3.68 MiB | 778.65 KiB |
d4f7e6b+dirty | 2.92 MiB | 3.67 MiB | 773.57 KiB |
aee9036+dirty | 2.92 MiB | 3.68 MiB | 777.93 KiB |
37635cb+dirty | 2.92 MiB | 3.67 MiB | 773.74 KiB |
…859-newCaptureFeedbackAPI-Form-logo
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
03c9048 | 500.96 ms | 486.65 ms | -14.31 ms |
561640f | 461.96 ms | 458.11 ms | -3.85 ms |
a3ba405 | 438.16 ms | 435.78 ms | -2.38 ms |
50c70c0 | 496.82 ms | 526.02 ms | 29.20 ms |
cadf235 | 462.20 ms | 463.34 ms | 1.14 ms |
f4a5053 | 478.22 ms | 458.35 ms | -19.87 ms |
1dd8d17 | 399.65 ms | 393.81 ms | -5.84 ms |
a06f6ba | 424.02 ms | 415.82 ms | -8.20 ms |
0781f75 | 452.32 ms | 457.22 ms | 4.91 ms |
e0624b6 | 447.67 ms | 441.08 ms | -6.59 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
03c9048 | 17.74 MiB | 20.10 MiB | 2.37 MiB |
561640f | 17.74 MiB | 20.09 MiB | 2.35 MiB |
a3ba405 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
50c70c0 | 17.74 MiB | 20.10 MiB | 2.36 MiB |
cadf235 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
f4a5053 | 17.74 MiB | 20.10 MiB | 2.36 MiB |
1dd8d17 | 17.74 MiB | 20.10 MiB | 2.36 MiB |
a06f6ba | 17.74 MiB | 20.09 MiB | 2.35 MiB |
0781f75 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
e0624b6 | 17.74 MiB | 20.10 MiB | 2.36 MiB |
Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
37635cb | 439.87 ms | 434.80 ms | -5.08 ms |
aee9036 | 469.91 ms | 466.33 ms | -3.58 ms |
e84c6bb | 470.61 ms | 462.14 ms | -8.48 ms |
d4f7e6b | 467.51 ms | 476.51 ms | 9.00 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
37635cb | 17.74 MiB | 20.10 MiB | 2.36 MiB |
aee9036 | 17.74 MiB | 20.10 MiB | 2.37 MiB |
e84c6bb | 17.74 MiB | 20.10 MiB | 2.37 MiB |
d4f7e6b | 17.74 MiB | 20.10 MiB | 2.36 MiB |
…859-newCaptureFeedbackAPI-Form-logo
Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
<Text style={styles.title}>{text.formTitle}</Text> | ||
{config.showBranding && ( | ||
<Image | ||
source={{ uri: 'https://sentry-brand.storage.googleapis.com/sentry-glyph-black.png' }} |
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.
Let's include logo locally.
I'm not sure who in Sentry owns this storage but it could be gone any time.
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.
That's a good point @krystofwoldrich 👍
I actually tried this (61e4d4d) but run into errors with packaging. I'll revisit my approach.
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.
Updated with 727a423 to include the logo as a base64 encoded image locally.
I've tried including it as a resource in the assets folder but run into issues with rendering on Android and the end packaging in the SDK.
…859-newCaptureFeedbackAPI-Form-logo # Conflicts: # packages/core/src/js/feedback/FeedbackForm.styles.ts # packages/core/src/js/feedback/FeedbackForm.tsx
…859-newCaptureFeedbackAPI-Form-logo
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.
Overall the code looks good! after checkinf if the smaller sized base64 image works, LGTM!
Thank you for the suggestion @lucas-zimerman 🙇
I pushed the change with c6dff03 Did you use a smaller image or another tool to created the smaller image? |
The first image had no transparent borders where the second one is the same image, both images I used https://tinypng.com/ to make it smaller. |
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 to me! lGTM!
Thank you for the tip 🙇 |
…859-newCaptureFeedbackAPI-Form-logo
265e629
into
antonis/3859-newCaptureFeedbackAPI-Form
* Update the client implementation to use the new capture feedback js api * Updates SDK API * Adds new feedback button in the sample * Adds changelog * Removes unused mock * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Directly use captureFeedback from sentry/core * Use import from core * Fixes imports order lint issue * Fixes build issue * Adds captureFeedback tests from sentry-javascript * Update CHANGELOG.md * Only deprecate client captureUserFeedback * Add simple form UI * Adds basic form functionality * Update imports * Update imports * Remove useState hook to avoid multiple react instances issues * Move types and styles in different files * Removes attachment button to be added back separately along with the implementation * Add basic field validation * Adds changelog * Updates changelog * Updates changelog * Trim whitespaces from the submitted feedback * Adds tests * Renames FeedbackFormScreen to FeedbackForm * Add beta label * Extract default text to constants * Moves constant to a separate file and aligns naming with JS * Adds input text labels * Close screen before sending the feedback to minimise wait time Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Rename file for consistency * Flatten configuration hierarchy and clean up * Align required values with JS * Use Sentry user email and name when set * Simplifies email validation * Show success alert message * Aligns naming with JS and unmounts the form by default * Use the minimum config without props in the changelog * Adds development not for unimplemented function * Show email and name conditionally * Adds sentry branding (png logo) * Adds sentry logo resource * Add assets in module exports * Revert "Add assets in module exports" This reverts commit 5292475. * Revert "Adds sentry logo resource" This reverts commit d6e9229. * Revert "Adds sentry branding (png logo)" This reverts commit 8c56753. * Add last event id * Mock lastEventId * Adds beta note in the changelog * Updates changelog * Align colors with JS * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Use regular fonts for both buttons * Handle keyboard properly * Adds an option on whether the email should be validated * Merge properties only once * Loads current user data on form construction * Remove unneeded extra padding * Fix background color issue * Fixes changelog typo * Updates styles background color Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Use defaultProps * Correct defaultProps * Adds test to verify when getUser is called * (2.2) feat: Add Feedback Form UI Branding logo (#4357) * Adds sentry branding logo as a base64 encoded png --------- Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Autoinject feedback form (#4370) * Align changelog entry * Update changelog * Revert "Autoinject feedback form (#4370)" This reverts commit da0e3ea. --------- Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
* Update the client implementation to use the new capture feedback js api * Updates SDK API * Adds new feedback button in the sample * Adds changelog * Removes unused mock * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Directly use captureFeedback from sentry/core * Use import from core * Fixes imports order lint issue * Fixes build issue * Adds captureFeedback tests from sentry-javascript * Update CHANGELOG.md * Only deprecate client captureUserFeedback * Add simple form UI * Adds basic form functionality * Update imports * Update imports * Remove useState hook to avoid multiple react instances issues * Move types and styles in different files * Removes attachment button to be added back separately along with the implementation * Add basic field validation * Adds changelog * Updates changelog * Updates changelog * Trim whitespaces from the submitted feedback * Adds tests * Adds attachment button UI * Adds changelog * Add attachment handling based on the client implementation * Reduce render method complexity * Adds test for attachment button visibility * Format code * Pick image with react-native-image-picker * Convert base64 string to Uint8Array before sending * Updates changelog * Renames FeedbackFormScreen to FeedbackForm * Add beta label * Extract default text to constants * Moves constant to a separate file and aligns naming with JS * Adds input text labels * Close screen before sending the feedback to minimise wait time Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Rename file for consistency * Flatten configuration hierarchy and clean up * Align required values with JS * Use Sentry user email and name when set * Simplifies email validation * Show success alert message * Aligns naming with JS and unmounts the form by default * Use the minimum config without props in the changelog * Adds development not for unimplemented function * Show email and name conditionally * Adds sentry branding (png logo) * Adds sentry logo resource * Add assets in module exports * Revert "Add assets in module exports" This reverts commit 5292475. * Revert "Adds sentry logo resource" This reverts commit d6e9229. * Revert "Adds sentry branding (png logo)" This reverts commit 8c56753. * Add last event id * Mock lastEventId * Remove changelog * Reverse unrelated change * Adds beta note in the changelog * Updates changelog * Align colors with JS * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Use regular fonts for both buttons * Handle keyboard properly * Adds an option on whether the email should be validated * Merge properties only once * Loads current user data on form construction * Remove unneeded extra padding * Fix background color issue * Fixes changelog typo * Updates styles background color Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> * Use defaultProps * Correct defaultProps * Adds test to verify when getUser is called * Add default value in doc comment Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Add a more clear doc comment Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * (2.2) feat: Add Feedback Form UI Branding logo (#4357) * Adds sentry branding logo as a base64 encoded png --------- Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com> * Autoinject feedback form (#4370) * Align changelog entry * Update changelog * Use AddScreenshot naming * Allow only Uint8Array for screenshots * Rename callback parameter * Adds snapshot tests for screenshot button * Rename screenshot button for clarity * Use a library to get the Uint8Array --------- Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
📢 Type of change
📜 Description
Based on #4328
Adds Sentry logo at the top of the form
💡 Motivation and Context
See #4328 (comment)
💚 How did you test it?
CI, Manual testing
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps
#skip-changelog