asciidoctor / asciidoctor-pdf

:page_with_curl: Asciidoctor PDF: A native PDF converter for AsciiDoc based on Asciidoctor and Prawn, written entirely in Ruby.
https://docs.asciidoctor.org/pdf-converter/latest/
MIT License
1.14k stars 500 forks source link

Allow theme to configure images for admonition blocks #1697

Closed PartTimeDataScientist closed 2 years ago

PartTimeDataScientist commented 4 years ago

While trying to figure a workaround for not getting font based icons to work in an OSGI environment I realized that icon names are ignored with :icons:image

admonition: icon: __note: __name: far-sticky-note __stroke_color: 111111 __size: 24

works perfectly fine with :icons: font, but gives warnings with :icons: image

D:>asciidoctor-pdf -a pdf-themesdir=D:/ -a pdf-theme=minimal Asciidoctor_Cheatsheet.adoc Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF-8 asciidoctor: WARNING: admonition icon not found or not readable: D:/images2/note.svg asciidoctor: WARNING: admonition icon not found or not readable: D:/images2/tip.svg asciidoctor: WARNING: admonition icon not found or not readable: D:/images2/caution.svg asciidoctor: WARNING: admonition icon not found or not readable: D:/images2/important.svg

Example files: Asciidoctor_Cheatsheet.adoc.txt minimal-theme.yml.txt

mojavelinux commented 4 years ago

This behavior is per design. The theme file configures the names of the font-based icons, not the image-based icons. You can configure the path of the image-based icon using the icon attribute on the block.

PartTimeDataScientist commented 4 years ago

Thanks for the information. I already configured a custom path using the :iconsdir: attribute which is what you are referring to, right? From your answer it sounds like there is is a rationale behind not "allowing" to define filenames for image icons so that it is probably needless to turn this into a feature request. Would you mind commenting on that?

I envisioned having a larger icons folder shared between documents (containing e.g. all the fontawesome .svg icons and more) and being able to choose icons per document by switching stylesheets. The current implementation would require multiple icon folders each containing different icons but all with the same names (note.svg, warning.svg, ...) which seems more difficult work with and to maintain.

As a side note: This would also address the initial question on this thread in the discussion forums.

mojavelinux commented 4 years ago

I think you've misunderstood my point. I'm not saying you cannot have a custom icon image for an admonition block. That's a standard part of AsciiDoc. What I'm saying that it's not what the theme is used for. The theme configures the font-based icon name. A custom icon image must be configured per admonition block.

:iconsdir: path/to/icons
:icons: image

[NOTE,icon=sticky-icon.svg]
====
This is a NOTE admonition with a sticky icon image.
====

We could consider supporting an image path in the theme for admonitions when image-based icons are set. We've just never had that request before.

PartTimeDataScientist commented 4 years ago

OK, thanks for clarifying that. To me that seems like a slightly different use-case. This feature allows one to put a special icon at a specific admonition block to highlight that. And as defining global icons is the default behavior for the font based icons it at least was surprising for me that it wasn't true for the image based icons.

Although I don't have too much experience with Ruby I'll try to have a look at the code and see if I can come up with a PR that transfers the behavior from the font based icons to the image based icons.

mojavelinux commented 4 years ago

Yep, that's why I'm saying it's a feature request. I'm going to update the title to reflect that.

Here's what I propose:

admonition:
  icon:
    note:
      image: image:sticky-note.svg[,24]

It's going to be tricky because once you introduce the option to specify an image, you need to do the full image macro processing (at least enough to get the path right and the width). I don't mind taking a stab at it.

mojavelinux commented 4 years ago

The current implementation would require multiple icon folders each containing different icons but all with the same names (note.svg, warning.svg, ...) which seems more difficult work with and to maintain.

For each different set of icons, you could specify iconsdir to point to the icons you want. It was for this reason we had never considered allowing the image filename to be configured globally. However, the downside is that it isn't currently linked with the theme. Neither strategy is right or wrong, so it's reasonable to support both.

PartTimeDataScientist commented 4 years ago

Here's what I propose:

admonition:
  icon:
    note:
      image: image:sticky-note.svg[,24]

Why would you introduce another option to define the icon size while the key admonition-icon-\<name>​-size is already available for themeing? (Re-)using that key it would be sufficient to simply set a filename to be searched for in the :iconsdir: folder (optionally adding :icontype: suffix to filename if it does not end with .svg or .png)

What do you think of

 admonition:
   icon:
     note:
       name: fas-sticky-note    # Font based icon name
       image: sticky-note.svg   # Image filename
       size: 24                 # Size for both options

which would allow easy switching between :icons: image and :icons: font in the document.

mojavelinux commented 4 years ago

Why would you introduce another option to define the icon size while the key admonition-icon-​-size is already available for themeing?

Because then when you switch between different icon modes (fonts or images), you cannot reuse the same theme (because the "size" will conflict...plus the size is a font size, not an image dimension). These are independent features and need to be configured separately.

mojavelinux commented 4 years ago

(Re-)using that key it would be sufficient to simply set a filename to be searched for in the :iconsdir: folder

I don't think the iconsdir folder should be used in this case. That makes the image macro work differently in this case than in other places in the theme...and that's confusing.

We need to be consistent with how the theme resolves images, so we will follow the logic already defined. See https://github.com/asciidoctor/asciidoctor-pdf/blob/master/docs/theming-guide.adoc#images

So the image will be resolved relative to the pdf-themesdir.

mojavelinux commented 4 years ago

Settings in the theme are not affected by settings in the document. These are two different things. Either you define the image for the admonition in the theme, using theme constructs, or you define the image in the AsciiDoc content (as we discussed), which uses iconsdir, icontype, etc. There's no overlap here.

PartTimeDataScientist commented 4 years ago

Looks like this topic is "slightly" more complex than I initially grasped and I also realize that there is much more to learn and understand regarding theming and what options there are to define different things. Thanks for taking the time to comment and explain, I really appreciate that!

Resolving (icon) image paths relative to the theme should work fine. Would require to distribute themes packed with the icons, but that is a solvable problem. On the long run one could consider to allow themes packed together with icons and images in zip-files but that is surely a different issue.

Nonetheless with my rudimentary ruby knowledge I better stay clear of the actual implementation and look for more efficient ways for contributing to and promoting the use of Asciidoc(tor)!

mojavelinux commented 2 years ago

This turns out to be less complex than we initially realized. The theming system already has support for specifying images. We can build on that by allowing the image to be specified for an admonition type, which is consulted when image-based icons are enabled:

admonition:
  icon:
    tip:
      image: lightbulb.svg

The image is resolved relative to the themedir by default. If you want to resolve it relative to the document directory instead, you can prefix the path with the {docdir} attribute reference:

admonition:
  icon:
    tip:
      image: '{docdir}/lightbulb.svg'

The image will then be positioned and rendered just as though you specified it using the icon attribute on the admonition block. In fact, under the covers, it's the same logic.

If the image is not found, the converter will log a warning and use the textual label instead.

I want to defer supporting the image macro as a value for now as I think it will create confusion.