Conversation
|
fixes #375 |
mukeshmandiwal
force-pushed
the
add-website-icon
branch
from
0f589f3 to
658f062
Compare
|
We can remove the 2nd commit that adds delectation message by rebasing with latest branch. The PR is easier to focus if it has only your contribution. |
docs/src/Root.js
Outdated
| <> | ||
| <Helmet> | ||
| <title>{title}</title> | ||
| <link rel="shortcut icon" type="image/png" href="/logo.png"/> |
There was a problem hiding this comment.
Logo in this case is 512 size, maybe a smaller 64x64 file for favicon.png or favigon.ico would be a better usage than logo.
mukeshmandiwal
force-pushed
the
add-website-icon
branch
from
658f062 to
bfe2beb
Compare
mukeshmandiwal
force-pushed
the
add-website-icon
branch
from
bfe2beb to
7f823fe
Compare
|
Maybe we should have a background color for dark mode? So that the favicon doesn't show black on dark gray like GitHub's favicon in macOS Chrome dark mode (screenshot below)?
|
|
Should we use logo-white.png as favicon ? |
|
Hm, that will have problems on non-dark mode tabs, wouldn't it? I guess my suggestion would be to fill in the transparent parts as well as a 2 pixel margin around it with white (2 pixels when it's shrunk to favicon size). Something that would look like this (this is not a great version, just threw it together in 2 minutes):
|
|
You may also want to consider generating some other versions of the favicon with something like RealFaviconGenerator (you could take a look at my PR here: keepassxreboot/keepassxc-org#54). |
|
I generated a new one and, it looks like this
|
| <> | ||
| <Helmet> | ||
| <title>{title}</title> | ||
| <link rel='shortcut icon' type='image/x-icon' href='/favicon.ico'/> |
There was a problem hiding this comment.
Thanks for looking into this! Whatever you end up using to generate the icon with, can you please add an npx script to the package.json? – see how repng is used for the other images
There was a problem hiding this comment.
I use https://realfavicongenerator.net/ link to generate favicon from static/logo.svg file
There was a problem hiding this comment.
Thanks! If there’s not an installable package this will likely fall out of sync over time
There was a problem hiding this comment.
As of my concern, there is no installable package which likely fulfills our requirements
There was a problem hiding this comment.
Maybe one of the following npm packages? I just did a quick search for "svg to ico npm" on Google:
There was a problem hiding this comment.
It could also be generated from the png output from repng, if that makes finding a library easier
There was a problem hiding this comment.
Is there any reason not to use a .png file for the favicon? I assume most modern browsers support this now, but haven't done any research
|
Thanks again for the work on this! I'll go ahead and merge this PR since I'm getting ready to ship updated docs for v5 |
4 participants
No description provided.