-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Font Awesome shortcode has some non-obvious behaviours #71
Comments
While it's possible to add FA5 by default with the new deferred block logic that has been added to core Grav 1.5+, it relies on the theme being updated to properly do so. This is because it will still break users that upgrade shortcode-core that do NOT have the deferred block setup in their theme that we have outlined as the new default approach: https://getgrav.org/blog/important-theme-updates I'm all for improving the FA5 support though. I have not really used it myself as it has not really added anything of note for the 'free/opensource' version. In fact, it really makes things more complicated and with additional shims to be compatible (which we need), it makes things slower and heavier. All the good stuff is in the paid version, unfortunately :( . I personally have bee using ForkAwesome, which is basically a forekd vesion of FA4, with new icons. |
Sorry for the delay in replying - I wrote a response but apparently didn't post it. I'm not sure that I can see how the deferred block logic is relevant here. Currently, the only thing that the option does is change the behaviour of the extras option - it doesn't change anything about what is being loaded, and it is entirely up to the user to ensure that they load the version that they want. Even if someone is using FA4, having support for FA5 enabled shouldn't cause an issue for them, unless they want to include a class called For clarity, the current behaviour is as follows: With FA5 disabled: With FA5 enabled: |
Just a sidebar @rhukster but I'd say FA5, even the free version, offers some serious improvements. The SVG + JS version is pretty "awesome" I think -- way more modern than using a font to represent an icon. Plus, with using tree shaking you can very include include only the icons you are using, have them inserted as SVGs. As such, you end up with a tiny file (~10kb) instead of a huge font file you are barely using. Also, a bonus of using the SVG + JS setup you can do very cool stuff like layering, text and counter overlays. |
Just wanted to add that if someone wants to use Fontawesome 5 aside from enabling it in the plugin's option menu you also need to change the Fontawesome URL from This also solved #81 for me, adding the missing icons. |
I've identified a few potential issues with the Font Awesome shortcode. Once I've had some feedback from you with regard to the desired behaviour, I will put together a PR that fixes them and adds support for the new duotone icons. If you would prefer each part of this as a separate PR, just let me know.
extras
prependsfa-
Currently the README only includes examples where classes in
extras
start withfa-
, with the exception ofmargin-bottom
, howeverfa-margin-bottom
does not appear to be a standard FA class. This implies both that it's necessary to give the full class name for FA classes, and that arbitrary classes can be specified here. Obviously, the documentation should be updated to reflect the functionality.Changing the current behaviour of
extras
is obviously impractical, just in case someone is using the shortcut, but should we add an additional option for including non-FA classes?FA5 support is disabled by default
It was noted in #46 that there are some compatibility issues between FA4 and FA5, and so when support for FA5 was added in #56, it was set to be disabled by default. Currently, the only effect of this option is to set the style (
$fa_class
) tofas
,far
,fal
, orfab
if those are entered inextras
. I'm unable to think of any realistic situation where this behaviour would be an issue for FA4 users, particularly since they would need to be explicitly included by the user (eg. they would have to addextras=fas
with the intention of getting the classfa-fas
).Does support for FA5 really need to be disabled by default?
The text was updated successfully, but these errors were encountered: