Skip to content
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 5 Needs 'far' or 'fas' classes, not 'fa' #29

Closed
1 of 2 tasks
coneybeare opened this issue Oct 18, 2018 · 7 comments
Closed
1 of 2 tasks

Font Awesome 5 Needs 'far' or 'fas' classes, not 'fa' #29

coneybeare opened this issue Oct 18, 2018 · 7 comments

Comments

@coneybeare
Copy link

I'm submitting a...

  • Bug report
  • Feature request

Reproduction steps

  1. install EasyMDE on a site that already uses FontAwesome, and thus doesn't pull the loaded one
  2. Look at toolbar's missing icons

Version information

Browser type and version:
EasyMDE version: 2.4.1

Font Awesome 5 has changed the way they do their classing from Font Awesome 4. No longer can you simply use fa, rather you need to use fas, far, etc... It appears that your autoloaded version actually uses FA 4.7.0, does not behave this way, so when using EasyMDE on a site that already uses FontAwesome 5, the icons do not show.

It seems to me that the solution is to simply add far or fas to the corresponding icon classes, as it won't affect people who still use FA4, and the extra fa won't affect people using FA5.

@Ionaru
Copy link
Owner

Ionaru commented Oct 18, 2018

Hi, thank you for your bug report.

I've tested with both the CSS and SVG&JS versions of FontAwesome 5 free and the FA5 icons render fine for me.

image

Can you provide an example that shows the issue?

@coneybeare
Copy link
Author

coneybeare commented Oct 18, 2018

I'm not sure what is accidentally going right in your tests, but the fa prefix is clearly deprecated in FA5, as per the official FontAwesome site. Have you tried using their FontAwesome 5 can instead of the v4.7.1 bootstrap one you are currently using?

Personally, I use the official font awesome sass gem to build font-awesome from scratch within a Rails deployment. I was hoping the official docs would be enough for you to fix this, but if not, I guess I could throw together a mini rails app you can setup and run if need be.

@coneybeare
Copy link
Author

Ok, I put together a local rails project and installed FontAwesome-sass. EasyMDE did not properly detect it, and instead loaded the bootstrapcdn's 4.7 version. The icons did work however, leading me to discover that the problem I was seeing above stems from a security header blocking font-src from allowing the bootstrapcdn's 4.7 version of FA.

So this issue can be closed, but it opens two new questions...

  1. Why isn't it detecting the FontAwesome I already have installed?
  2. Why are you still loading a V4 version of FA instead of V5 from their official CDN?

@Ionaru
Copy link
Owner

Ionaru commented Oct 18, 2018

  1. That's was an oversight of me when I made the editor FA5 compatible.
  2. Because the editor still uses FA4 by default and I did not want to change too much of the editor at the time.

But to possibly solve your FA4 issue: you can prevent the editor from downloading FA4 by setting the autoDownloadFontAwesome option to false.

@philgyford
Copy link

Should the README be updated so that the SimpleMDE fork section says "FontAwesome 4 compatibility" rather than "FontAwesome 5 compatibility"?

Until I found this issue I was confused as to why FontAwesome 5 styles didn't work, and then why v4.7 was being downloaded, when the README says EasyMDE uses v5.

@Ionaru
Copy link
Owner

Ionaru commented Nov 16, 2021

The README is correct, the editor has compatibility with FontAwesome 5 (Five), that was the initial reason for the fork. However the default is still FontAwesome 4.

Look at https://easy-markdown-editor.tk/ for an example of the editor running using FontAwesome 5.
Keep in mind that you will need to provide the FontAwesome 5 script yourself, for example https://use.fontawesome.com/releases/v5.15.4/js/all.js.

@philgyford
Copy link

Thank you for the clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants