Skip to content
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

Open
wants to merge 6 commits into
base: antonis/3859-newCaptureFeedbackAPI-Form
Choose a base branch
from

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Dec 11, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Based on #4328

Adds Sentry logo at the top of the form

showBranding=true showBranding=false
iOS ios-logo ios-no
Android android-logo android-no

💡 Motivation and Context

See #4328 (comment)

💚 How did you test it?

CI, Manual testing

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

#skip-changelog

@antonis antonis mentioned this pull request Dec 11, 2024
10 tasks
Copy link
Contributor

github-actions bot commented Dec 11, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 401.50 ms 421.26 ms 19.76 ms
Size 7.15 MiB 8.38 MiB 1.23 MiB

Baseline results on branch: antonis/3859-newCaptureFeedbackAPI-Form

Startup times

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

Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo

Startup times

Revision Plain With Sentry Diff
d4f7e6b+dirty 404.04 ms 454.62 ms 50.58 ms

App size

Revision Plain With Sentry Diff
d4f7e6b+dirty 7.15 MiB 8.38 MiB 1.23 MiB

Copy link
Contributor

github-actions bot commented Dec 11, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.93 ms 1227.58 ms 12.65 ms
Size 2.36 MiB 3.11 MiB 761.04 KiB

Baseline results on branch: antonis/3859-newCaptureFeedbackAPI-Form

Startup times

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

Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo

Startup times

Revision Plain With Sentry Diff
d4f7e6b+dirty 1243.76 ms 1249.80 ms 6.04 ms

App size

Revision Plain With Sentry Diff
d4f7e6b+dirty 2.36 MiB 3.11 MiB 761.07 KiB

Copy link
Contributor

github-actions bot commented Dec 11, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.04 ms 1237.52 ms -0.52 ms
Size 2.92 MiB 3.67 MiB 773.60 KiB

Baseline results on branch: antonis/3859-newCaptureFeedbackAPI-Form

Startup times

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

Previous results on branch: antonis/3859-newCaptureFeedbackAPI-Form-logo

Startup times

Revision Plain With Sentry Diff
d4f7e6b+dirty 1240.88 ms 1236.92 ms -3.96 ms

App size

Revision Plain With Sentry Diff
d4f7e6b+dirty 2.92 MiB 3.67 MiB 773.57 KiB

Copy link
Contributor

github-actions bot commented Dec 12, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 467.51 ms 476.51 ms 9.00 ms
Size 17.74 MiB 20.10 MiB 2.36 MiB

Baseline results on branch: antonis/3859-newCaptureFeedbackAPI-Form

Startup times

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

@antonis antonis marked this pull request as ready for review December 13, 2024 11:20
<View style={styles.titleContainer}>
<Text style={styles.title}>{text.formTitle}</Text>
{config.showBranding && (
<Image
Copy link
Collaborator

@lucas-zimerman lucas-zimerman Dec 13, 2024

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)

Copy link
Collaborator Author

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?

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.

2 participants