search

jessedoyle / prawn-icon

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

Make the icon compliant with Font Awesome HTML classes #53

Closed Slokilla closed 2 years ago

Slokilla commented 2 years ago

Actually, to add an icon, we have to use a custom name “faX-XXXX”. I think that it could be cool if the lib was able to parse the FA syntax used in HTML classes “faX fa-XXXX”.

An exemple of use is : I have a DB with a table that have a column with an icon. I stock the icon as a string : the FA class name. The main purpose would be to use the same DB entry to generate my web pages and my PDFs without a lot a string processing.

I imagine two possible implementations :

  1. \faX fa-XXXX\
  2. \fa-XXXX\ ==> This one look to be a breaking change, but way more user_friendly imho
jessedoyle commented 2 years ago

Hey @Slokilla thanks for using Prawn::Icon!

The example use case provided is interesting, but I think it's possible to look at it a different way as well.

Instead of having a single DB column (i.e. icon), the app could have 2 columns (i.e. icon_name and icon_family). That way it would be trivial to concatenate the values and create a valid icon identifier in both Prawn::Icon and HTML.

The primary concern I have with this type of implementation is that it opens the door for more error cases. For example:

These are all things that can be solved, but it takes a fair bit of introspection to be able to be able to handle these errors in a friendly way. I'm not sure the complexity is worth it.

I'd like to avoid making any breaking changes to Prawn::Icon unless absolutely necessary.

Though I would consider merging a pull request that implements this suggestion in a non-breaking way. One thought would be to use a "shim" that performs a direct mapping of the HTML-compliant icon to the internal Prawn::Icon specifier.

This could also be implemented outside of prawn-icon and could live in middleware or another Ruby gem that your app pulls in (thinking on it - maybe this is the best option).

I hope the reasoning here makes sense, I'm open to a bit of discussion on the subject!

Slokilla commented 2 years ago

Ok, your arguments are reasonnable, I will reevaluate the needs to make a PR for this. Feel free to reopen the issue if you need to talk again about this :)

jessedoyle commented 2 years ago

Thanks for the suggestion @Slokilla!

If you do end up creating a library that implements this, please share it! I'd be happy to link to it from Prawn::Icon's README.