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

HDDS-9572. Restore favicons from original ozone website #51

Closed

Conversation

makinyemi
Copy link

This PR adds favicons from original site, and update docusaurus.config.js to point to the location of the new favicon

Restored favicons from original size. Pointed to new location in docusaurus config
@errose28 errose28 added the website-v2 Tasks for the new ozone website targeting the HDDS-9225-website-v2 branch label Nov 13, 2023
@errose28 errose28 self-requested a review November 13, 2023 18:35
Copy link

@dombizita dombizita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @makinyemi, I'm not too familiar with it, but I think in this patch only the favicon.ico is actually used, none of the other files that you added in static/img is used anywhere in the code. Does it get picked up somehow? I tried your changes locally and I removed everything, except static/img/favicon.ico and the change in docusaurus.config.js, where you are adding the favicon. In the browser the favicon was still there. I also tried checking the Docusaurus documentation, it seems only one file can be added there, so it's different from the approach in the current site, that was done here. What do you think?

@makinyemi
Copy link
Author

@dombizita correct, the other images are not used in this patch. My assumption was they would be used in some future patch so my thought was to add all the previous existing favicons from the current site in this patch.

I tried your changes locally and I removed everything, except static/img/favicon.ico and the change in docusaurus.config.js, where you are adding the favicon.

I had updated the favicon.ico in this patch and may have been experiencing some caching issues that didnt allow me to see the changes immediately. In messing around with the docusaurus.config.js it appeared to have finally updated with my change, but looking at the docs again I was wrong. I will revert that back

@errose28
Copy link
Contributor

Hi @makinyemi thanks for working on this. Turns out this is tricky to get right and after spending the better part of a day on it this is what I came up with:

  • Docusaurus does not support multiple favicons out of the box, so to add multiple icons we need to use custom head tags.
  • Favicons work best when they are at the root of the website. It seems in some instances browsers check <website>/favicon.ico instead of the header.
    • To keep the numerous favicon files from being confused with other static site files, we can use static directories in docusaurus to put them in a unique location
  • favicon.ico is the most universally supported format. SVG is nice to have but not as well supported.
  • The site.webmanifest file is for Progressive Web Applications (PWAs), which have their own requirements for icons.
    • A 512x512 PNG icon
    • A 192x192 PNG icon
    • A "maskable" versions of these icons that are centered with a background. I generated these with https://maskable.app/editor from our existing SVG logo and the background to the current social card.

I have all of this on this branch of my fork. I tested it using this chrome extension on the local website preview. If it looks good I can either push it to this PR or open a new PR.

References:

@errose28
Copy link
Contributor

Actually it looks like docusaurus has a PWA plugin. We should probably use this to set up everything except the main .ico favicon and svg favicon.

@errose28
Copy link
Contributor

I've taken another stab at this in #95

@errose28
Copy link
Contributor

Since #95 is merged, I'm going to close this for now. If there are other improvements to favicons we want to do we can do them in a follow up PR.

@errose28 errose28 closed this Jun 21, 2024
@makinyemi makinyemi deleted the HDDS-9572-favicons branch August 28, 2024 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
website-v2 Tasks for the new ozone website targeting the HDDS-9225-website-v2 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants