-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
logo data uri encoding #1527
logo data uri encoding #1527
Conversation
no longer need to replace `+` with `%2B`
Generated by 🚫 dangerJS |
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.
This change looks good to me! Two observations though:
- would a short comment be useful? I can foresee some head scratching when I look at this piece of code again in six months time! 😄
- could you add an additional case in the existing test function to make sure this change is actually covered?
Good idea! makeLogo()
when given 'gratipay' and {"logo":"image/svg+xml;base64,PHN2ZyB4bWxu"}
✓ should return 'data:image/svg+xml;base64,PHN2ZyB4bWxu'
when given 'gratipay' and {"logo":"data:image/svg+xml;base64,PHN2ZyB4bWxu"}
✓ should return 'data:image/svg+xml;base64,PHN2ZyB4bWxu'
when given 'gratipay' and {"logo":"data:image/svg xml;base64,PHN2ZyB4bWxu"}
✓ should return 'data:image/svg+xml;base64,PHN2ZyB4bWxu' |
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.
I had a similar test in mind, it should do the trick! The comment is helpful as well. Well done! 😉
Thanks for reviewing @PyvesB 😄 |
currently you need to replace any base64 logos
+
characters with%2B
in order for the logo to show correctly:Examples:
using

+
:+
→%2B
:This PR removes that extra step by replacing any whitespace
\s
with a+
character.As any
+
characters in the query param get replaced with a space.