Skip to content

FEATURE: Implement component for displaying Favicons in frontend#176

Open
crydotsnake wants to merge 6 commits intoneos:8.4from
crydotsnake:feature/implement-favicons
Open

FEATURE: Implement component for displaying Favicons in frontend#176
crydotsnake wants to merge 6 commits intoneos:8.4from
crydotsnake:feature/implement-favicons

Conversation

@crydotsnake
Copy link
Member

No description provided.

@mhsdesign
Copy link
Member

Thanks for your work ^^ Is it normal that this seems so complex / verbose?

I mean this is supposed to be a exemplary demo - im not sure if this wouldnt scare people away :P

... But if this is the way. I would not oppose.

image

@crydotsnake
Copy link
Member Author

crydotsnake commented Jul 20, 2023

Well. You could also use a fluid file with the HTML for the favicons. And a fusion file that inherits from Neos.Fusion:Template and just has the package and the Fluid file as the templatePath as propertys.

But the idea as I have solved it here came to me as the first thought.

But both solutions are not complex at all IMO 🤔

Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

Today you don't need so many icons anymore, see https://evilmartians.com/chronicles/how-to-favicon-in-2021-six-files-that-fit-most-needs or other articles. Also the manifest should be adjustable or have safe placeholders, as it contains something about Sandstorm and Neos and people might just copy it.
To make the manifest adjustable, similar to the Robots.txt in Neos.Seo would actually be a cool example I think. Also I would not use the ResourceUri but directly use the StaticResource Eelhelper in the Tag. They should anyway be a bit more individual and not all have the 32px size attribute.

@crydotsnake
Copy link
Member Author

Today you don't need so many icons anymore, see https://evilmartians.com/chronicles/how-to-favicon-in-2021-six-files-that-fit-most-needs or other articles.

First i thought about using a favicon generator. For example: https://realfavicongenerator.net/

But was not sure if this would be a good solution. Wdyt?

@Sebobo
Copy link
Member

Sebobo commented Jul 21, 2023

Why not just remove the tags you don't need and keep the rest and just do a few adjustments?
Don't fully understand your question regarding the generator and how it would help?

@Sebobo
Copy link
Member

Sebobo commented Sep 16, 2024

Should target 8.4 though

@crydotsnake crydotsnake changed the base branch from 8.3 to 8.4 September 16, 2024 12:54
@crydotsnake
Copy link
Member Author

Should target 8.4 though

Done.

@crydotsnake crydotsnake requested a review from Sebobo June 23, 2025 06:46
@crydotsnake crydotsnake requested a review from Sebobo June 23, 2025 14:32
Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

LGTM, thx

Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

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

Thanks @crydotsnake for this change. Is there a reason for having a different logo variant in the size 70x70?

@crydotsnake
Copy link
Member Author

Thanks @crydotsnake for this change. Is there a reason for having a different logo variant in the size 70x70?

I used a favicon generate which maybe was not the best idea 🤔 I believe this alternative is for dark modes for microsoft edge. But i'm not 100% sure. I used https://realfavicongenerator.net/ i belive..

@markusguenther
Copy link
Member

Hmm, in the end, we don’t need that many-sized PNG files.
We can use one SVG for the modern browsers and could also use the CSS query for dark mode with that if we want. But as the demo is not using dark mode stuff at all, we don’t need it either.
https://blog.tomayac.com/2021/07/21/dark-mode-web-app-manifest-app-icons/

So my suggestion would be to add one SVG and a PNG as a fallback for the devices that do not use it yet.
WDYT?

@crydotsnake
Copy link
Member Author

Hmm, in the end, we don’t need that many-sized PNG files. We can use one SVG for the modern browsers and could also use the CSS query for dark mode with that if we want. But as the demo is not using dark mode stuff at all, we don’t need it either. https://blog.tomayac.com/2021/07/21/dark-mode-web-app-manifest-app-icons/

So my suggestion would be to add one SVG and a PNG as a fallback for the devices that do not use it yet. WDYT?

Sounds fine to me. Which site would you use to create the favicon png and svg variant?

@markusguenther
Copy link
Member

Ok maybe ico as fallback as windows is an issue ;)

ICO: Universally supported across all browsers and platforms, including very old ones.
PNG: Supported by most modern browsers, but not all platforms (e.g., Windows taskbar prefers .ico).

@markusguenther
Copy link
Member

We have all SVG icons and just need to use the ico then, but bet we already have a ico file in the project.
https://github.com/neos/brand/tree/master/logos/Neos

@markusguenther
Copy link
Member

Here you have already an ICO Resources/Public/Assets/Favicons/favicon.ico

@crydotsnake crydotsnake self-assigned this Jun 24, 2025
@crydotsnake
Copy link
Member Author

@markusguenther Can you take a look again?

Copy link
Member

@jonnitto jonnitto left a comment

Choose a reason for hiding this comment

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

It would be better if the favicon has an aspect ratio of 1:1

@markusguenther
Copy link
Member

Would have chnaged it, but was not allowed to push.

@crydotsnake
Copy link
Member Author

Would have chnaged it, but was not allowed to push.

Maybe because of my fork. If you want, I can invite you to my fork repo.

@crydotsnake
Copy link
Member Author

Would have chnaged it, but was not allowed to push.

Maybe because of my fork. If you want, I can invite you to my fork repo.

@markusguenther ping ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Status: Reviews

Development

Successfully merging this pull request may close these issues.

5 participants