-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 icons not working after v2.6.0 upgrade #1522
Comments
The issue can be replicated outside of MagicMirror e.g. FA4 css first and fa5 css first Whilst this shows this issues is in itself not a MagicMirror bug, it is triggered by MagicMirror allowing both versions of Font Awesome stylesheets to be included in the generated html. Furthermore, it doesn’t look like either the end user (via |
This is a tough one. We might want to add a generic config which sets the desired FA-version. The only issue is that a module can't depend on either one of the libs being available. Any recommendations in a solution? |
Quite a challenge indeed! On the assumption that the goal is to adopt FA5 as the single FA library within MM and given FA do not recommend running FA4 and FA5 side by side in one project the path I would recommend is
All other options seem to rely on vendor specific work arounds within the core MM code which, depending on users configuration and developer support could trigger a variety of outcomes and support headaches. I appreciate introducing such a roadmap presents some problems, but it does seem the cleanest way forward. The main challenge is how to force the inclusion of the v4-shim if a module references to “fontawesome.css” in getStyles without requiring changes to the module code. i.e. include two css files for one getStyles reference. |
I understand your approach, but I don't want to introduce a roadmap with breaking changes. Simplicity is key for MagicMirror². We can probably support both FA versions by namespacing them to specific classes. If a module has the |
Totally support the principle and will continue searching for solutions. I guess I was put off by the official statement and conversations such as this one on stackoverflow citing similar issues with WordPress plugins. To avoid (minimise) breaking changes, is it possible to move to step 2 (FA5 + v4 shim) now, but not plan for step 3? Quick tests with a modified vendor file where “fontawesome.css” installs fa5, a dummy plugin which getstyles for the fa4 shim included last in the config and reverting getstyles in the default calendar module seem to work. This gives backwards compatibility and leaves the adoption of fa5 in the hands of plugin devs based on how they wish to support the new prefixes (fas, fab, far etc) |
A shim sounds perfect. Feel free to send a PR. |
Fix conflict between font awesome versions as per #1522
Merged! |
Hi @MichMich
Great work as ever getting another update out - hope you managed to celebrate the New Year whilst completing the release!
I have uncovered an issue with the Font Awesome 5 support introduced in v2.6.0
MagicMirror Version: v2.6.0
Description:
When amodule includes
"fontawesome.css"
(i.e. font awesome 4) and another module (e.g. the default calendar) includes"font-awesome5.css", "font-awesome5.v4shims.css"
then unexpected results can occur depending on which order they are included inconfig.js
.Steps to Reproduce:
A: Modify
config.js
file in a v2.6.0 install so it only includes definitions for the default calendar module and then MMM-Strava (in that order). In this instancefa-hashtag
andfa-trophy
display as expected, butfa-arrows
(a renamed icon) does not.B: Modify
config.js
file in a v2.6.0 install so it only includes definitions for the default calendar module and then MMM-Strava (in that order). Change the default config for calendar to usesymbol: "volleyball-ball"
. In this instancefa-hashtag
andfa-trophy
display as expected, butfa-arrows
(a renamed icon) andfa-volleyball-ball
(a new icon) do not.C: Change
config.js
to include definitions for MMM-Strava then the default calendar module (in that order) and all icons appear.D: Change
config.js
to omit the default calendar module (i.e. only include MMM-Strava) and all icons appear.Expected Results:
All fa4 and fa5 icons should appear in all instances
Actual Results:
If the fa5 (e.g. calendar) module is omitted from
config.js
, the fa4 module works as expected.If the fa5 (e.g. calendar) module is added to
config.js
before the fa4 module, the config for the fa5 module cannot use new fa5 icons AND the fa4 module cannot use renamed iconsAdditional Notes:
The release notes for v2.6.0 suggest the new FA5 functionality is backwards compatible. However, it feels like this is either a bug or a breaking change.
A workaround is to release an update to the fa4 module so that any icons are renamed per the font awesome 4 upgrade notes. However, to do so means the third party module will depend on v2.6.0 (i.e.
requiredVersion
must be increased to2.6.0
) thus preventing existing users who choose not to upgrade from taking advantage of improvements to a module.Also, if another fa4 module is added to the config after the upgraded fa5 modules, the problem recurs.
The text was updated successfully, but these errors were encountered: