-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Feedback form UI Branding logo #4357
base: antonis/3859-newCaptureFeedbackAPI-Form
Are you sure you want to change the base?
Feedback form UI Branding logo #4357
Conversation
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 385.30 ms | 433.06 ms | 47.76 ms |
cadf235+dirty | 455.51 ms | 451.64 ms | -3.87 ms |
0781f75+dirty | 406.72 ms | 454.80 ms | 48.08 ms |
a3ba405+dirty | 359.67 ms | 436.86 ms | 77.19 ms |
a06f6ba+dirty | 381.50 ms | 429.77 ms | 48.27 ms |
26fc306+dirty | 382.83 ms | 435.31 ms | 52.48 ms |
561640f+dirty | 378.73 ms | 442.25 ms | 63.52 ms |
27e1bf3+dirty | 398.69 ms | 439.39 ms | 40.69 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
50c70c0+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
cadf235+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
0781f75+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
a3ba405+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
a06f6ba+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
26fc306+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
561640f+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
27e1bf3+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a3ba405+dirty | 1223.00 ms | 1219.06 ms | -3.94 ms |
26fc306+dirty | 1227.25 ms | 1225.85 ms | -1.40 ms |
27e1bf3+dirty | 1230.92 ms | 1232.33 ms | 1.41 ms |
a06f6ba+dirty | 1230.45 ms | 1227.09 ms | -3.36 ms |
0781f75+dirty | 1222.19 ms | 1222.11 ms | -0.08 ms |
50c70c0+dirty | 1228.06 ms | 1224.43 ms | -3.64 ms |
561640f+dirty | 1220.45 ms | 1227.02 ms | 6.57 ms |
cadf235+dirty | 1223.89 ms | 1236.22 ms | 12.33 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a3ba405+dirty | 2.36 MiB | 3.11 MiB | 760.99 KiB |
26fc306+dirty | 2.36 MiB | 3.11 MiB | 761.18 KiB |
27e1bf3+dirty | 2.36 MiB | 3.11 MiB | 761.03 KiB |
a06f6ba+dirty | 2.36 MiB | 3.11 MiB | 761.35 KiB |
0781f75+dirty | 2.36 MiB | 3.11 MiB | 761.35 KiB |
50c70c0+dirty | 2.36 MiB | 3.11 MiB | 760.92 KiB |
561640f+dirty | 2.36 MiB | 3.11 MiB | 761.19 KiB |
cadf235+dirty | 2.36 MiB | 3.11 MiB | 761.47 KiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a3ba405+dirty | 1229.31 ms | 1228.16 ms | -1.14 ms |
26fc306+dirty | 1229.10 ms | 1227.88 ms | -1.22 ms |
27e1bf3+dirty | 1245.78 ms | 1244.38 ms | -1.40 ms |
a06f6ba+dirty | 1235.31 ms | 1238.76 ms | 3.45 ms |
0781f75+dirty | 1247.90 ms | 1237.11 ms | -10.79 ms |
50c70c0+dirty | 1226.61 ms | 1225.02 ms | -1.59 ms |
561640f+dirty | 1237.10 ms | 1229.59 ms | -7.51 ms |
cadf235+dirty | 1225.19 ms | 1231.65 ms | 6.47 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a3ba405+dirty | 2.92 MiB | 3.67 MiB | 773.65 KiB |
26fc306+dirty | 2.92 MiB | 3.67 MiB | 773.77 KiB |
27e1bf3+dirty | 2.92 MiB | 3.67 MiB | 773.54 KiB |
a06f6ba+dirty | 2.92 MiB | 3.67 MiB | 773.87 KiB |
0781f75+dirty | 2.92 MiB | 3.67 MiB | 773.83 KiB |
50c70c0+dirty | 2.92 MiB | 3.67 MiB | 773.48 KiB |
561640f+dirty | 2.92 MiB | 3.67 MiB | 773.72 KiB |
cadf235+dirty | 2.92 MiB | 3.67 MiB | 773.97 KiB |
…859-newCaptureFeedbackAPI-Form-logo
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
561640f | 461.96 ms | 458.11 ms | -3.85 ms |
a06f6ba | 424.02 ms | 415.82 ms | -8.20 ms |
27e1bf3 | 463.19 ms | 478.80 ms | 15.61 ms |
0781f75 | 452.32 ms | 457.22 ms | 4.91 ms |
50c70c0 | 496.82 ms | 526.02 ms | 29.20 ms |
cadf235 | 462.20 ms | 463.34 ms | 1.14 ms |
26fc306 | 426.80 ms | 421.58 ms | -5.22 ms |
a3ba405 | 438.16 ms | 435.78 ms | -2.38 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
561640f | 17.74 MiB | 20.09 MiB | 2.35 MiB |
a06f6ba | 17.74 MiB | 20.09 MiB | 2.35 MiB |
27e1bf3 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
0781f75 | 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 |
26fc306 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
a3ba405 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
…859-newCaptureFeedbackAPI-Form-logo
Co-authored-by: LucasZF <[email protected]>
<View style={styles.titleContainer}> | ||
<Text style={styles.title}>{text.formTitle}</Text> | ||
{config.showBranding && ( | ||
<Image |
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.
Can we use an svg image?
from here: https://sentry.io/branding/
we could have an SVG image, like this one:
<svg class="css-lfbo6j e1igk8x04" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 44" width="400" height="352" style=""><path d="M29,2.26a4.67,4.67,0,0,0-8,0L14.42,13.53A32.21,32.21,0,0,1,32.17,40.19H27.55A27.68,27.68,0,0,0,12.09,17.47L6,28a15.92,15.92,0,0,1,9.23,12.17H4.62A.76.76,0,0,1,4,39.06l2.94-5a10.74,10.74,0,0,0-3.36-1.9l-2.91,5a4.54,4.54,0,0,0,1.69,6.24A4.66,4.66,0,0,0,4.62,44H19.15a19.4,19.4,0,0,0-8-17.31l2.31-4A23.87,23.87,0,0,1,23.76,44H36.07a35.88,35.88,0,0,0-16.41-31.8l4.67-8a.77.77,0,0,1,1.05-.27c.53.29,20.29,34.77,20.66,35.17a.76.76,0,0,1-.68,1.13H40.6q.09,1.91,0,3.81h4.78A4.59,4.59,0,0,0,50,39.43a4.49,4.49,0,0,0-.62-2.28Z" fill="#362d59"></path></svg>
I like the idea of using the svg icon because it's easy to customize the logo color, so if a user decides to use a dark theme, the Sentry logo will not be hidden (by that we would need to allow them to change the sentry color or maybe some logic to know if the background color is dark or bright)
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. I defaulted to PNG due to the discussion with @krystofwoldrich on the form PR.
I think that using an svg directly might require a library which I'm not sure we want to import in core 🤔
Maybe I can try using a JSX with a convertor like https://www.svgviewer.dev/svg-to-react-jsx. Wdyt?
📢 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