Allow option to use locally loaded version of the library

[herbdool]

This would allow for some customization of the icon set.

[bugfolder]

FYI, we've had some discussion about the possibility of bundling FontAwesome with Bootstrap 5 Lite module. Here's the issue it's discussed in. TL:DR; bundled FontAwesome is huge.

[yorkshire-pudding]

The full set is huge, but people could slim it down to just the icons they need. Perhaps this was the customisation @herbdool had in mind. It is the use case I'm thinking about.

[yorkshire-pudding]

I've just done it using a nodejs tool (that took a long time to figure out) to shrink the font files. I've also slimmed down the css file to only cover what I need, and added it to the theme so the CSS could be aggregated.

So we should also include a theme option that doesn't try to load any separate CSS. This would also work if the contrib theme added the icons.

One might ask, why bother with the module in that case, but quite a few other modules check for the existence of Font Awesome module.

[yorkshire-pudding]

Hi @bugfolder and @herbdool

I've added the option for both a locally defined path and also for the library to be added as part of the theme. The main reason for the latter option is that on a site I run where they wanted to go to the nth degree on optimisation, I created a smaller font file and a css doing that, then added via SCSS to the the regular CSS.

I've tested both these options. Any feedback? I'd like to release at the same time as the v6 addition.

[bugfolder]

Hi @yorkshire-pudding, I've tried it out. Adding the library via a local path is a great option. I'd suggest adding some discussion to the README to explain what that means (you need to download the library yourself, you can use a slimmed-down version if you only need a few icons, etc.).

I'm having a little difficulty to understand the difference between "Disabled" and "Theme - add as part of the theme." Functionally, it looks like both settings have the effect of "Don't add FontAwesome via this module."

Maybe some explanation of "Disabled" to include the case where FontAwesome is already included by the theme?

Note that with radio button groups, you can add separate description text under each button, so perhaps for Disabled and Local (and Theme, if that option persists) you could add some explanation as description text for those options.

[yorkshire-pudding]

Hi @bugfolder - thank you for the feedback and suggestions. Bizarrely, I had never noticed the disabled option, but you are right that disabled has the same action as 'theme'

Didn't know that could add descriptions to radio buttons but that suggestion makes sense.

[yorkshire-pudding]

@bugfolder I've updated the PR to make things clearer:

• Changed disabled to 'None' and added description
• Added description to Local
• Changed 'Theme' to Other and added description
• Moved links from option to description at top of form
• Added explanation to readme and clarified.

[bugfolder]

@yorkshire-pudding, those are all great changes! I do have one more suggestion. Now that the radio buttons have description, on Seven, the "v4 shims" checkbox looks like it's part of the list of radio buttons:

Wrapping the radios in a fieldset (and then using the fieldset title, and leaving the 'radios' title empty) provides a little better clarity that the "v4 shims" option is something else:

And perhaps move the usage description above the fieldset:

[yorkshire-pudding]

Great suggestions @bugfolder . I've done details rather than fieldset as that is recommended by the docs.

[bugfolder]

Looks good!