tomfran / typo

A simple Hugo theme
https://tomfran.github.io/
MIT License
247 stars 76 forks source link

feat(partials): allow setting of favicon path #16

Closed mesmur closed 3 months ago

mesmur commented 3 months ago

I was trying to set up the favicon for my hugo website but was having no luck with it.

The wiki simply told me to copy my favicon files into the static directory but for some reason that just led to no favicon at all.

When inspecting the head.html partial I saw that there was no <link> tag setting the favicon (I'm not too experienced in frontend so I'm not sure how the default favicon was being set at all?)

This PR should make the favicon setup explicit, it also allows setting the path based on preference using a param. The fallback is set to the favicon.ico value which should be current state (except made explicit) so as to not make this a breaking change

tomfran commented 3 months ago

Thank you for the PR :)

Perhaps it's browser-dependent, what are you using not to have them displayed? Anyway, better specify them as you did.

Can you please include also extra icons for IOS and Android? As in the static folder we also have the following:

So we should include them all:

<link rel="icon" type="image/ico" href="/favicon.ico">
<link rel="icon" type="image/png" sizes="16x16" href="/favicon-16x16.png">
<link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png">
<link rel="icon" type="image/png" sizes="192x192" href="/android-chrome-192x192.png">
<link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon.png">

Having a single param for each of those would be inappropriate, maybe you can specify a base folder path and hardcode the filenames. This way a user could nest the icons in a static subfolder and specify the subpath in the config.

Tell me if this makes sense.

mesmur commented 3 months ago

Perhaps it's browser-dependent, what are you using not to have them displayed?

Ah I tried on both arc (chromium) and firefox, but wasn't able to get things working even after resetting my cache.

Tell me if this makes sense.

Makes perfect sense!

I introduce a param faviconPath that the user can set as the base folder path. The filenames were hardcoded as you suggested, I agree with that decision.

An example would be:

faviconPath = 'img' which would target the /img folder in the built public folder.

Leaving it empty would default to /, which is the current behavior and should be backwards compatible.


Also my auto-formatter made some minor changes in other sections to keep the style consistent. Let me know if that's not ok and I will revert those changes

tomfran commented 3 months ago

Looks good, thank you!

achanda commented 3 weeks ago

Sorry if this is obvious, but I could not figure out how to set faviconPath. I tried setting it to ./static/favicon.ico but that did not work. I checked that my static dir does have favicon.ico, what am I missing?