jessedoyle / prawn-icon

Easy icons for Prawn.
Other
28 stars 15 forks source link

mapping for font-awesome 4 solid circle fa-circle not mapped #61

Closed npiper closed 1 year ago

npiper commented 1 year ago

Hi,

Am using a buckeyball style ( thin circle, contrast circle, full circle) gradient, but on my asciidoctor-pdf conversion the solid circle is not rendering, looks like not in the mapped list? in /data/fonts/fa4/shims.yml?

would it just be to add:

fa-circle: far-circle far-solid

Or is the solid harder to map?

FA5 icon ref: solid circle https://fontawesome.com/icons/circle?s=solid&f=classic

Code sample asciidoc:

.Example: Maturity Assessment Table
[%autowidth.stretch]
|===
|Maturity  | Description | Assessment 

| icon:circle-o[] 
|
| 

| icon:adjust[]
|
| icon:check-square-o[]

| icon:circle[]
| 
|

|===

PDF Output: (Last row expected to be solid circle)

Screenshot 2022-11-18 at 23 12 33
npiper commented 1 year ago

or?

fa-circle: fas-circle

Could use the full icon list from fa4 for unit / compatabiliity test; (tag 4.7.0) https://raw.githubusercontent.com/FortAwesome/Font-Awesome/v4.7.0/src/icons.yml

jessedoyle commented 1 year ago

Hello @npiper - thanks for using prawn-icon!

The FontAwesome 4/5 compatibility shims aren't something we're looking to add to right now. They were intended to be a temporary stopgap measure to allow for non-breaking upgrades.

Ideally the font icon set should be explicitly configured for the fas family. Looking at the docs, there appears to be a couple of ways to do this.

Does something like this work for you?

:icons: font

icon:circle@fas[]
npiper commented 1 year ago

Thanks for tip - worth a quick addition to this article as a tip to the README?

This worked to create the output for full circle on PDF output but means you need slightly different code between asciidoc HTML output vs PDF output.

 icon:circle[set=fas]

Would be a nice addition if community could bring compatability of the fontawesome mapping up to 4.7 for consistency when using asciidoctor-pdf, imagine they aren't doing any more fontawesome releases to that major version so would hopefully be last one.

jessedoyle commented 1 year ago

Hmm, I definitely understand the argument about portability for HTML/PDF. I'm not an AsciiDoctor expert though, so there could be a way to do this that's compatible for both.

Perhaps with the icon-set directive?

:icons: font
:icon-set: fas

icon:circle

Curious if @mojavelinux would have any suggestions as to best practice here?

With that being said, I can confirm that the fa-circle shim functions correctly when using this library standalone. We default to the fas family when unspecified.

For example, given the following code:

require 'prawn/icon'

Prawn::Document.generate('circle.pdf') do |pdf|
  pdf.icon 'fa-circle' # reference the icon via compatibility shim
end

We get a PDF document with the solid circle icon:

circle

It's also worthwhile to note that prawn-icon prints a pretty loud warning about the shim usage:

[Prawn::Icon - DEPRECATION WARNING]
  FontAwesome 4 icon was referenced as 'fa-circle'.
  Use the FontAwesome 5 icon 'fas-circle' instead.
  This compatibility layer will be removed in Prawn::Icon 4.0.0.

Not to pass the buck here, but I believe the issue ultimately lies in the asciidoctor-pdf repo. I think the best path forward would be to open an issue there.

Going to close this issue as the the fa-circle shim is actually working when used standalone - thanks!

mojavelinux commented 1 year ago

Asciidoctor PDF is already clear about how to handle this situation. See https://docs.asciidoctor.org/pdf-converter/latest/icons/#font We're not looking to make any changes to how it works. It is by design.

jessedoyle commented 1 year ago

Thanks @mojavelinux!

I think there's a slight difference in how asciidoctor-pdf resolves a legacy FontAwesome reference here.

The method body could be slightly altered to something like:

def resolve_legacy_icon_name name
  ::Prawn::Icon::Compatibility.new(key: "fa-#{name}").translate
end

That should keep the shim characteristics identical between prawn-icon and asciidoctor-pdf. Happy to submit a PR if that makes sense!

mojavelinux commented 1 year ago

My understanding is that's what this already does:

::Prawn::Icon::Compatibility::SHIMS[%(fa-#{name})]

The reason we don't use the translate method is because we don't want a secondary warning that doesn't go through Asciidoctor's logger.

It does appear as though we're missing a test because this broke at some point.

mojavelinux commented 1 year ago

"circle" was never a valid FontAwesome 4 icon. And Asciidoctor PDF is doing the right thing here. It's falling back to looking for the name in the icon-set set on the document, which is far by default. And in far, the circle is a disc (that's just how it is in Font Awesome). If you want the default fallback icon set to be fas, you must set it on the document.

mojavelinux commented 1 year ago

"circle" was never a valid FontAwesome 4 icon

That's actually not correct.

https://fontawesome.com/v4/icon/circle

So what I see is that prawn-icon doesn't actually implement the whole SHIM map. Rather, it must be assuming that fas is the fallback. However, that's not technically correct since that would mean we support many more icons when the set is "fa" than FontAwesome 4 ever did. So really, a complete mapping would be ideal. Otherwise, there's no way to know for sure.

jessedoyle commented 1 year ago

It's falling back to looking for the name in the icon-set set on the document, which is far by default.

Ah, that does make sense!

Just an FYI that the warning can be squelched by passing a dummy IO to the translate method.

i.e.

require 'stringio' # likely already loaded

def resolve_legacy_icon_name name
  ::Prawn::Icon::Compatibility.new(key: "fa-#{name}").translate(StringIO.new)
end
mojavelinux commented 1 year ago

That's good to know!

...still, I prefer more more low-level control for the very careful migration we're trying to achieve in Asciidoctor PDF.

jessedoyle commented 1 year ago

When we implemented the FontAwesome 5 shim originally, I recall pretty testing it quite thoroughly.

This shims in this repo were derived directly from the first party FontAwesome shim metadata: https://github.com/FortAwesome/Font-Awesome/blob/0a4935e849b1ae1360d03b84eaf84a4a205e9ebb/advanced-options/metadata/shims.yml

I might not be remembering this correctly, but I believe the first-party shims work off of the assumption that the solid family (fas) is the default family when performing key translation.

mojavelinux commented 1 year ago

Again, the problem is if fas is the fallback, we have no way of knowing whether it is falling back to a previously mapped icon or a new icon in Font Awesome 5. And if I allow that fall-through, then we have people asking why an icon that works in Asciidoctor PDF isn't found in the HTML output.

However, given that we are doing a fallback after the warning, I could see falling back to fas instead of what is set on the document. That seems like a reasonable change. But I would prefer not to fall back at all in this case.

npiper commented 1 year ago

Suggestion worked around my display difference using the full notation, but If needed for future ref - Did a quick comparison delta of explicit mapped id's for the Prawnicon FA4 Shim file vs. the FontAwesome full 4.7 Icon list:

Sources:

Fontawesome 4.7.0 icons def yml https://raw.githubusercontent.com/FortAwesome/Font-Awesome/v4.7.0/src/icons.yml

Prawnicon fa4 --> fa5 Shims.yml https://raw.githubusercontent.com/jessedoyle/prawn-icon/master/data/fonts/fa4/shims.yml

Delta of 214 not explicitly mapped - might mean fallback behavior for these. (461 in Shim map vs 675 in FA4.7 full icon set)

Excel analysis attached, also added Open office & CSV format: fa4.7_delta_prawniconFA4.csv fa4.7_delta_prawniconFA4.ods fa4.7_delta_prawniconFA4.xlsx

fa4.7_delta_prawniconFA4_DELTA.csv

mojavelinux commented 1 year ago

Here's the approach I decided to take: https://github.com/asciidoctor/asciidoctor-pdf/pull/2374/files I think I managed to get all Font Awesome 4 icons mapped so that we can both select the right ones and validate icons which are not in the Font Awesome 4 set.