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

Moved favicons to site root; font-awesome and highlight.js now served from CDN #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abhivs
Copy link

@abhivs abhivs commented Aug 5, 2015

Apologies for bundling these two loosely related changes; I ended up implementing them together in my production site using this theme to avoid hosting any duplicate files.

For simplicity and wider cross-browser compatibility, I moved the favicon/iOS icon files to the root directory of the website from the images directory and updated the HEAD to reflect the new location.

I also removed the highlight.js file and all of the font-awesome files (stylesheet and fonts) in favor of serving those standard libraries from an external CDN. This simplifies the site's directory structure, saves a good amount of space, and in many cases decreases page load times.

The github.css file (code highlight styles for highlight.js) could probably also be removed and served from a CDN if the background-color edit in the file was moved to main.css.

@ethanmad
Copy link
Contributor

ethanmad commented Aug 5, 2015

I'm not an expert on this, but doesn't loading the files from an external
CDN allow the CDN to track visitors?

Also, it may be beneficial to restructure the commits such that the favicon
changes are one commit and the resource changes are another.

@abhivs
Copy link
Author

abhivs commented Aug 5, 2015

As far as I know, it would be technically possible to track visitors using remotely hosted JavaScript (in this case, highlight.js), but such behavior would be improbable and easy-to-detect. The CDNs I selected have excellent reputations for reliability and security (both support HTTPS, even).

Yeah, in case Keichi wants to implement one change but not the other, the commits would have to be restructured. Otherwise, though, I don't think it's a huge issue.

@ethanmad
Copy link
Contributor

ethanmad commented Aug 5, 2015

OK, I suppose the risk is worth the gain in load times. Better not to duplicate their efforts.

Regarding the favicons, what are the improvements in compatibility? The root directory gets really cluttered with so many icons (when they could be nicely tucked away in /icons).

@abhivs
Copy link
Author

abhivs commented Aug 5, 2015

It's annoying from an organizational perspective, I agree. To ensure that all browsers and web services see the best possible version of the favicons, though, apparently they must be in the root directory and favicon.ico should not be declared in HTML. This FAQ does a good job of explaining it.

IMO, the minor organizational annoyance is worth it to be in keeping with browser standards--even if they boil down to just semantics in the end.

@icecreammatt
Copy link
Contributor

I currently keep my favicon in the root of the site. I keep all the variations in the icon folder and it seems to work pretty well. I used the tool from the site listed above to generate mine. It had me put them all in an icons folder with a manifest. I didn't have to move the apple icons to the root directory though. That all said I have not tested it on all platforms.

Here are the changes I made:

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

Successfully merging this pull request may close these issues.

3 participants