Skip to content

put svg with xmlns in img src tag#1538

Merged
mortenpi merged 11 commits into
JuliaDocs:masterfrom
jkrumbiegel:inline-svg-in-img
Mar 17, 2021
Merged

put svg with xmlns in img src tag#1538
mortenpi merged 11 commits into
JuliaDocs:masterfrom
jkrumbiegel:inline-svg-in-img

Conversation

@jkrumbiegel
Copy link
Copy Markdown
Contributor

@jkrumbiegel jkrumbiegel commented Feb 26, 2021

This fixes #1537

A small test I made with Luxor, to show that inline svgs are correctly scaled down to fit the column width:

```@example
using Luxor
@svg begin
    background("black")
end 300 300
```

```@example
using Luxor
@svg begin
    background("red")
end 800 800
```

```@example
using Luxor
@svg begin
    background("blue")
end 1500 1500
```

before:

grafik

after:

grafik

Comment thread src/Writers/HTMLWriter.jl Outdated
Comment thread src/Writers/HTMLWriter.jl Outdated
@mortenpi
Copy link
Copy Markdown
Member

Could you also include a few additional test cases here: https://github.com/JuliaDocs/Documenter.jl/blob/master/test/examples/src/man/tutorial.md#including-images-with-mime

It would be good to have a small matrix of cases there, e.g. whether the <svg> has a viewBox or not, if it has width/height or not etc. Quickly reading through the article you linked, it sounded like even with img tags there might be edge cases where the SVG doesn't scale quite right?

@mortenpi mortenpi added Format: HTML Related to the default HTML output Type: Enhancement labels Feb 27, 2021
@jkrumbiegel
Copy link
Copy Markdown
Contributor Author

It would be good to have a small matrix of cases there

Would that be purely visual tests, checked manually? Because I don't think I can write a test that detects a browser's ability to show the embedded svg :)

@jkrumbiegel
Copy link
Copy Markdown
Contributor Author

jkrumbiegel commented Feb 28, 2021

I've made the following changes now:

SVG stays utf-8, with minimal changes to keep it from corrupting the surrounding html. I think that can also be nice if something doesn't look quite as expected, one can still inspect the SVG string manually in the browser, which doesn't work with base64. And base64 seems to inflate the size anyway.

I splice in the xmlns tag if it's not there, I couldn't find reasons why I shouldn't.

I've added a couple test cases on the page you linked.

@jkrumbiegel
Copy link
Copy Markdown
Contributor Author

I'd like to bump this as Makie's documentation could really use a simpler way to return svg's from example blocks

Copy link
Copy Markdown
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

Sorry @jkrumbiegel, this dropped off my radar. Let's get this wrapped up.

Comment thread src/Writers/HTMLWriter.jl Outdated
@mortenpi mortenpi added this to the 0.26.4 milestone Mar 14, 2021
Copy link
Copy Markdown
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

LGTM. Do you mind using Documente#master for Makie for a while though? I would like to leave this change for 0.27, but I don't want to rush that. This can easily be done if you commit the docs/Manifest.toml.

Comment thread src/Writers/HTMLWriter.jl Outdated
@mortenpi mortenpi modified the milestones: 0.26.4, 0.27.0 Mar 14, 2021
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
@jkrumbiegel
Copy link
Copy Markdown
Contributor Author

No I don't mind using master, thanks for helping to push this over the finish line.

@devmotion
Copy link
Copy Markdown
Contributor

I experienced the same problem with Jupyter notebooks, so maybe it would be useful to add the same fix to IJulia?

xref: https://discourse.julialang.org/t/cairomakie-and-fontconfig/55931/17?u=devmotion

@mortenpi
Copy link
Copy Markdown
Member

Thanks for iterating on this @jkrumbiegel!

@mortenpi mortenpi merged commit a7df180 into JuliaDocs:master Mar 17, 2021
@cormullion
Copy link
Copy Markdown
Contributor

Did this PR fix the "text in SVG images gets corrupted" issue? Still seeing this problem...

@jkrumbiegel
Copy link
Copy Markdown
Contributor Author

Yes, but it's only on master for now

@cormullion
Copy link
Copy Markdown
Contributor

OK, thanks (and for the fix!). I'll put my doc updates aside for a while...

@mortenpi
Copy link
Copy Markdown
Member

I'll put my doc updates aside for a while...

If you have some docs that need this now, I would suggest just using #master for now by committing a docs/Manifest.toml file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Format: HTML Related to the default HTML output Type: Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use URI-encoding for SVG images coming from at-example blocks

4 participants