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

Fix encoding % sign breaking SVG background #1097

Merged
merged 1 commit into from
Sep 12, 2020

Conversation

qw-in
Copy link
Contributor

@qw-in qw-in commented Sep 10, 2020

This is a quick fix for the backgrounds breaking - guess I'll have to spend more time with this whole encoding issue than initially anticipated. Really hoping to find some time to put together those tests but it's been sunny here 😅

Escaping the % sign was breaking some of the styles, specifically percentage units. But only for SVG 😢

Closes #1095

Copy link
Contributor

@mfix22 mfix22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qw-in really appreciate you looking in to this. Would love to look into a solution that avoids the extra option.

function escapeXhtml(string) {
return string.replace(/%/g, '%25').replace(/#/g, '%23').replace(/\n/g, '%0A')
function escapeXhtml(string, escapePercentSign) {
if (escapePercentSign) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qw-in I'm pretty sure that instead of passing this in as a prop, we can just update the code path that converts to PNG to escape the percent sign, and don't convert that in the path used by SVGs.

@mfix22 mfix22 added the bug:fix label Sep 12, 2020
@mfix22 mfix22 merged commit 035dd32 into carbon-app:main Sep 12, 2020
@mfix22
Copy link
Contributor

mfix22 commented Sep 12, 2020

@qw-in merged this to get the fix out the door, but feel free to come back to simplify it. I would prefer to avoid boolean options like escapePercent if possible. But if it is not, no worries.

Really appreciate your continued help here! It is a huge help to Carbon 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Carbon SVG Export has Broken Backgrounds
2 participants