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: blank space on top/bottom when export image/svg #567

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

nhymxu
Copy link

@nhymxu nhymxu commented Jan 6, 2022

📑 Summary

Remove blank space on top/bottom when export image/svg

Resolves #566

📏 Design Decisions

Before export image, svg. Remove height attribute on svg element.
So browser when re-calculate new fitable height.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 🔖 targeted develop branch

@netlify
Copy link

netlify bot commented Jan 6, 2022

✔️ Deploy Preview for mermaidjs ready!

🔨 Explore the source changes: 8675947

🔍 Inspect the deploy log: https://app.netlify.com/sites/mermaidjs/deploys/61d6bda13e5f9e0007a50605

😎 Browse the preview: https://deploy-preview-567--mermaidjs.netlify.app

@@ -14,6 +14,7 @@
svg?.setAttribute('width', `${width}px`); // Workaround https://stackoverflow.com/questions/28690643/firefox-error-rendering-an-svg-image-to-html5-canvas-with-drawimage
if (!svg) {
svg = document.querySelector('#container svg');
svg.removeAttribute('height');
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if removing the height attribute would cause any other issues?

Copy link
Author

Choose a reason for hiding this comment

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

@sidharthv96

More detail I posted on issue.
When you not have height, browser will re-calculate new height from element inside svg element.

So, diagram will fit inside.

When edit new code, height will back to large number. This code block affect only for save action

You can test with data I put in issue

Copy link
Member

@sidharthv96 sidharthv96 Jan 6, 2022

Choose a reason for hiding this comment

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

This is what I get after downloading SVGs. Check the dimensions in bottom left.

After fix

image

Before fix

image

So this fix seems to break SVG downloads.

Also, these are 2 PNGs I downloaded (Firefox, macOS).
So is the issue OS/Browser specific?

After Fix

mermaid-diagram-20220106190925

Before Fix

mermaid-diagram-20220106190919

Copy link
Author

Choose a reason for hiding this comment

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

I'm using macOS. I'm using chrome on issue screenshot.

Now, try again with private window on Firefox

image

Can you see big blank space?

And here is exported png

mermaid-diagram-20220106215724

Copy link
Author

Choose a reason for hiding this comment

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

Here is dimmension

image

Maybe problem only happend on macOS?

I don't have any device with other OS here to test 😢

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so the issue is not browser/OS, but rather window size.
I was using my external monitor previously. When the window is smaller, the issue appears.

Can we brainstorm and come up with a proper solution?

image

Copy link
Author

Choose a reason for hiding this comment

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

Yes, If you have any idea, we can discuss.
My solution only fix for export function. But I think problem from render function first.

Copy link
Author

Choose a reason for hiding this comment

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

Some VSCode plugin support preview mermaid diagram have same problem, so I think it's from main mermaid.js. Not about live editor

@github-actions
Copy link

This pr is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

This pr is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 14 days

@github-actions github-actions bot added the Stale label Jun 9, 2022
@sidharthv96 sidharthv96 added retained Will not be auto-closed and removed Stale labels Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
retained Will not be auto-closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants