riboseinc / jekyll-theme-open-project

Jekyll theme for site suite presenting your open software and specifications.
MIT License
30 stars 3 forks source link

Font Awesome not working #64

Closed coder2000 closed 5 years ago

coder2000 commented 5 years ago

CDN being used for Font Awesome is for users who have paid for Pro. It should probably be switched to the free version or have a config switch.

ronaldtse commented 5 years ago

Thanks @coder2000 !

@strogonoff could you help take care of this? Thanks!

strogonoff commented 5 years ago

Thanks for reporting! Next bugfix release (should be this week) planning to make the way FA gets included more flexible.

On 1 Apr 2019, at 5:55 PM, Ronald Tse notifications@github.com wrote:

Thanks @coder2000 !

@strogonoff could you help take care of this? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

strogonoff commented 5 years ago

@ronaldtse

I am not satisfied in the solution currently in pull request(s), I think it’s too complex.

We have the following situation (feel free to skip to my proposed solution below):

The default Reasons to change Change how
Include all FA styles Performance (may be loading unused icons) Include only specific styles
Free version (upcoming default) More icons for custom styling Use Pro version
Include latest FA version at the time of theme release New or updated icons Specify FA release
Use official FA CDN Performance, intranet, China, etc. Host FA files on my own hosting or another CDN

I think the best way is to make it zero-configuration by default, and if any customization is needed then the user sets fontawesome_cdn: no and specifies custom <script> tags inside a template include.

Let me know if there’s something I’m missing, otherwise I’m going to build upon the PRs and change the following in upcoming release:

ronaldtse commented 5 years ago

Maybe I'm misunderstanding, but right now it is zero configuration and defaults to the free version.

In any case, the suggestions you made are excellent, so please help go ahead with them 👍

strogonoff commented 5 years ago

Great! I stand corrected, zero-configuration as the default is done right in the PRs, only the (optional) configuration method doesn’t feel 100% right.

strogonoff commented 5 years ago

We did end up using one icon that is not supported in free in any style (external-link-square), but its alternative version is indistinguishable and available in solid.

Merging Ronald’s branch with following changes:

strogonoff commented 5 years ago

Going to include in 2.0.2 release

strogonoff commented 5 years ago

The new triple-dot does doesn’t really work as well, but I’ll leave it for now to bug me to redesign expandable documentation navigation UX planned anyway.

Before:

Screen Shot 2019-04-07 at 10 52 03 AM

After:

Screen Shot 2019-04-07 at 10 51 00 AM

It escaped my notice that we did in fact use light FA styles before, and they all now became solid

strogonoff commented 5 years ago

I’ll reopen this to remind myself to double-check for possible degraded looks after icon style change and adjust measurements if necessary

strogonoff commented 5 years ago

Should be all good.