asciidoctor / asciidoctor-kroki

Asciidoctor.js extension to convert diagrams to images using Kroki!
https://kroki.io/
MIT License
149 stars 50 forks source link

Title attribute is not used as the alt value on the <img> tag (Ruby) #382

Closed Curtisjk closed 2 years ago

Curtisjk commented 2 years ago

I am looking to set alt text for accessibility reasons on generated diagrams. We originally put a human-readable description in the target, but this gets put into the filename and nginx generally doesn't like spaces in URIs.

I can see in the code there is provision to set alt text using attr.title but I am not sure if this is supported fully? I attempted the following:

[plantuml,target-name,svg,title="Title Alt Text"]
....
Bob -> Alice : hello
....

But this still resulted in the following HTML:

<div class="content">
<img src="images/target-name-648[...].svg" alt="target-name">
</div>
<div class="title">Figure 1. Title Alt Text</div>

Any guidance here would be greatly appreciated :)

ggrossetie commented 2 years ago

Hey, are you using the Ruby or JavaScript library?

ggrossetie commented 2 years ago

Using the browser extension, it produces:

<div class="imageblock kroki">
<div class="content">
<img src="https://kroki.io/plantuml/svg/eNpzyk9S0LVTcMzJTE5VsFLISM3JyQcAPHcGKw==" alt="Title Alt Text">
</div>
<div class="title">Figure 1. Title Alt Text</div>
</div>
Curtisjk commented 2 years ago

We are using the version provided by the docker-asciidoctor container - which I believe is the ruby version?

ggrossetie commented 2 years ago

That's a bug we are getting and removing the title from attrs:

https://github.com/Mogztter/asciidoctor-kroki/blob/288b6974d48815b7032546eec0ed001b64780612/ruby/lib/asciidoctor/extensions/asciidoctor_kroki/extension.rb#L151

And then, we are resolving the alt value:

https://github.com/Mogztter/asciidoctor-kroki/blob/288b6974d48815b7032546eec0ed001b64780612/ruby/lib/asciidoctor/extensions/asciidoctor_kroki/extension.rb#L197-L205

But attrs['title'] is gone 😓

Feel free to open a pull request to fix this issue. We should also add a test case:

https://github.com/Mogztter/asciidoctor-kroki/blob/288b6974d48815b7032546eec0ed001b64780612/ruby/spec/asciidoctor_kroki_spec.rb#L9-L22

ggrossetie commented 2 years ago

And we should probably add some documentation about this logic 😉

Curtisjk commented 2 years ago

Thanks @Mogztter - I will work on the PR later this week.