widgets\Menu.php $iconClassPrefix should not be static

[CyberPunkCodes]

Issue:

This doesn't work with the latest FontAwesome v5 icons because a lot of the icons don't start with fa fa-xxx. Some start with fab or fas.

Here are a few examples:

fab fa-accessible-icon
fas fa-align-justify

File: widgets\Menu.php Line: 31

Currently is: public static $iconClassPrefix = 'fa fa-';

Should be: public $iconClassPrefix = 'fa fa-';

Then the corresponding fix on line 84 to this:

: '<i class="' . $this->iconClassPrefix . $item['icon'] . '"></i> ',

Reason:

I am using the latest FontAwesome icons from https://fontawesome.com/icons?d=gallery&m=free loaded via their CDN. So I am not using your built in, out-dated version of FontAwesome.

Implementing this simple fix, would allow us to continue using your plugin along with the latest FontAwesome.

Yes, we would have to set the $iconClassPrefix to '', an empty string, and manually pass the prefixes ourself. Like so:

<?= Menu::widget([
    'options' => ['class' => 'sidebar-menu'],
    'iconClassPrefix' => '',
    'items' => $items,
]) ?>

This would only take a couple minutes to update, until you get the rest of this package updated.

[schmunk42]

As asked over here https://github.com/dmstr/yii2-adminlte-asset/issues/154#issuecomment-373436906 - would it be better to remove the font-awesome dependency altogether?

[CyberPunkCodes]

My detailed reply: https://github.com/dmstr/yii2-adminlte-asset/issues/154#issuecomment-403913046

Yes, remove the font-awesome dependency, use the CDN.

I want to address first, that the use of icon in the Menu::widget() should still be there. To not add 1,000 issues and make the sky fall on everyone, the iconClassPrefix should still include the fa fa- by default. Just remove the static from it so we can override it to use FontAwesome v5.

The v4 fa class is deprecated, which means all of v4 is. My answer in the other issue, gives light on some of the major changes you would need to address for this update.

FontAwesome is an extremely popular library. If you used the CDN (even the new v5 one), odds are the user already has it in their cache anyways. So it would be better for everyone to use the CDN. I don't understand why people still use it as a dependancy. Things like jQuery, Bootstrap, FontAwesome, all should be using the official CDN links. If you need to modify them, overwrite them with a custom stylesheet loaded after it.

Your fix would be 2 staged.

First, update your repo to use the latest v4 CDN and remove it from composer. This would reduce people creating new issues, so you can move to the v5 and not look back. I would add the new classes so it supports v5 (fab, fal, far, fas as I mentioned) somewhat in the meantime, for those of us who want v5 before you get it rolled out. So we only have to load the v5 stylesheet and update the html icons.

Second, create a new major build version for v5 (no longer supporting v4) and update the code accordingly. The new version has new classes for the same icons. Nowhere in the CSS should you specifically target the font-family (or the unicode content) as there are a few of them now for the free/pro versions, brands, etc. I would even drop targeting the fa class in the CSS. Make this new build for v5 only.

If they have the pro version, they change the URL for the stylesheet in the head section so they can use their pro icons. All icons should be used within the HTML (ie: <i class="fas fa-bars"></i>).

This makes it so there are only 2 things a person would need to do if they needed a different FA version (version number, or pro). Update their template to the stylesheet in the header and modify the html to reflect the icon. No more composer, rebuilding (if combining/minifying), etc. Simple.

[schmunk42]

PRs welcome :)

... but not via CDN, please - we won't use CDN by default due to security issues.

[adummy832]

FAv5 please :))

[schmunk42]

We've removed the FontAwesome dependency in 3.0.0-alpha1 (just create a release) so you can pick your own Font Icon assets.