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

add proper svg viewbox #1390

Closed
wants to merge 2 commits into from
Closed

add proper svg viewbox #1390

wants to merge 2 commits into from

Conversation

pitwegner
Copy link

Contributes to #405

@ggrossetie
Copy link
Member

ggrossetie commented Nov 19, 2022

Thanks for your contribution!

Would you mind adding a test? You can take a look at: https://github.com/yuzutech/kroki/blob/main/mermaid/test/convert-test.js

Also, I think that this change should be done upstream: https://github.com/jgraph/drawio
Since diagrams.net is closed to contributions, we should create an issue.

@eikef
Copy link

eikef commented Jun 2, 2023

Missing this patch currently makes the diagramsnet integration via kroki in GitLab broken/unusable. With this patch, diagrams can actually be embedded.

Upstream appears to have had this as well: jgraph/drawio#332

However, that has since been refactored into GraphViewer.js which does not seem to be handling the way kroki uses drawio for SVG exports; I have no idea how it actually works, that is just a guess.

@ggrossetie
Copy link
Member

For reference, we are currently using version v16.2.4 which is not the latest but still quite recent (Jan 7, 2022).

The issue you've mentioned was close Oct 23, 2018. It might be unrelated or we are missing a step with our integration to make sure that diagrams.net set the viewport.

I can try to update to the latest version of diagrams.net and see if the viewport is added.

However, that has since been refactored into GraphViewer.js which does not seem to be handling the way kroki uses drawio for SVG exports; I have no idea how it actually works, that is just a guess.

Kroki opens https://github.com/yuzutech/kroki/blob/main/diagrams.net/assets/index.html (using Puppeteer) and execute:

return render({
xml: source,
format: 'svg'
})

The render function is provided by diagrams.net.

@eikef
Copy link

eikef commented Jun 3, 2023

I tried using durrent -dev branch and, assuming I did everything correctly (it was a quick hack copying files according to the README) it did not help. On the upside, it worked approximately the same as before -- but I did not do many tests other than checking whether the viewbox issue was fixed.

I do not know how to fix it, all I know is that the patch here fixed that particular issue with embedding in GitLab and I thought it would be a good idea to add that context.
My hopes went up with that 2018 patch, but down when I saw it was 2018, and it ultimately bore no fruit.

@ggrossetie
Copy link
Member

Fixed in b195d56

before

<svg xmlns="http://www.w3.org/2000/svg" style="left: 0px; top: 0px; width: 100%; height: 100%; display: block; min-width: 82px; min-height: 82px;">
  <g>
    <g/>
    <g>
      <g transform="translate(0.5,0.5)" style="visibility: visible;">
        <ellipse cx="41" cy="41" rx="40" ry="40" fill="rgb(255, 255, 255)" stroke="rgb(0, 0, 0)" pointer-events="all"/>
      </g>
    </g>
    <g/>
    <g/>
  </g>
</svg>

after

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="82px" height="82px" viewBox="-0.5 -0.5 82 82">
  <defs/>
  <g>
    <ellipse cx="40" cy="40" rx="40" ry="40" fill="rgb(255, 255, 255)" stroke="rgb(0, 0, 0)" pointer-events="all"/>
  </g>
</svg>

@ggrossetie ggrossetie closed this Jun 3, 2023
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.

3 participants