Skip to content

Update prawn-icon to version 4.1.0#2550

Open
pepijnve wants to merge 1 commit intoasciidoctor:v2.3.xfrom
pepijnve:prawn_icon_4_1
Open

Update prawn-icon to version 4.1.0#2550
pepijnve wants to merge 1 commit intoasciidoctor:v2.3.xfrom
pepijnve:prawn_icon_4_1

Conversation

@pepijnve
Copy link
Copy Markdown
Member

This PR updates prawn-icon to v4.1.0

The main change in 4.1.0 is the upgrade of Font Awesome to v6. An extra feature has been added that allows to keep the icon size consistent even though the font metrics of FAv6 are a bit different to those of v5.

There are still two failing visual comparison tests. I wasn't sure if I could just update the reference. FWIW, the computer sees a difference, my eyes don't.

@mojavelinux
Copy link
Copy Markdown
Member

There are still two failing visual comparison tests. I wasn't sure if I could just update the reference. FWIW, the computer sees a difference, my eyes don't.

That's typical of these visual comparison tests. Sometimes it's a rounding difference that has no perceivable visual impact. What I typically do is check to see if it looks right, then just commit the updated reference file.

@mojavelinux
Copy link
Copy Markdown
Member

I needed to make sure that upgrading to FontAwesome6 was not a breaking change. Here's what the docs say:

Upgrading should be easy. But Downgrading could be difficult. Version 6 has some new icons, styles, and styling goodies. Once you’ve started using them in your project, downgrading to Version 5 may result in missing icons and styling.

So it appears that this upgrade is not a breaking change and thus should not require a major release. We're cheating a bit by putting it in a patch release, but we don't plan to make more minor releases of this major version line, so I'm willing to accept it.

@mojavelinux
Copy link
Copy Markdown
Member

Hmm, I now realize that we've been intentionally holding prawn-icon back in the v2.3.x branch for reasons of compatibility. The main branch used 3.1.0 while the v2.3.x branch uses 3.0.0. I really wonder whether this change should be made in the main branch instead. That might mean we need to accelerate the release of Asciidoctor PDF 3.0.0.

See #2507

@mojavelinux
Copy link
Copy Markdown
Member

Sometimes it's a rounding difference that has no perceivable visual impact.

I see what it is. Some of the icons have slightly different weights after the upgrade.

After seeing that, I'm convinced this change needs to target Asciidoctor PDF 3.0.0 (the main branch).

I'm also a little concerned about the two assertion changes regarding the font size. I understand that the font size has to be adjusted to achieve the icon height...but I'd like to verify that visually that means the output is identical as before the upgrade.

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.

2 participants