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

Favicon notification badge does not render correctly #210

Closed
federicobond opened this issue Nov 23, 2023 · 14 comments
Closed

Favicon notification badge does not render correctly #210

federicobond opened this issue Nov 23, 2023 · 14 comments

Comments

@federicobond
Copy link

I have some code laying around that implements this well, if you are interested I can submit a pull request for you to evaluate.

Captura de pantalla 2023-11-23 a la(s) 13 53 54
@axllent
Copy link
Owner

axllent commented Nov 23, 2023

Yes thank you, I'd be very interested to see what you have @federicobond. The library I'm currently using is fairly old, but was the best one I could find at the time that didn't suffer from bad performance, and which (I thought anyway) rendered ok on all major browsers. Out of curiosity, what browser and platform are you using?

@federicobond
Copy link
Author

That's Firefox on MacOS Sonoma 14.1. I'll look for some time these days to put something together.

@federicobond
Copy link
Author

Thanks for the quick answer!

@axllent
Copy link
Owner

axllent commented Nov 24, 2023

Thank you too. I realise that implementing a PR may be the most difficult thing for you, so even if you could point me in the direction of a better library that would be great (ie: I can implement it myself).

@federicobond
Copy link
Author

Sure, I put together a quick demo here: https://github.com/federicobond/badgicon

Feel free to copy and paste the code there and adapt it to your needs, I release every right to you.

@axllent
Copy link
Owner

axllent commented Nov 25, 2023

Thank you for this, I will definitely have a good look when I get some time (next weekend as I am away for the next 5 days).

In the meantime, could you please explain to me what the advantage is of your library vs: Tinycon (the one Mailpit uses)? I also had a quick play in my browser and has to bump the font size from 28 to 38 to get it similar to Mailpit's one as 28 was just too small to read, so I am wondering if it is just a font selection issue (ie: you don't have Arial installed).

@axllent
Copy link
Owner

axllent commented Dec 1, 2023

I have made a small change to the existing implementation to add a fallback font (sans-serif) to see if this helps your problem (I suspect you simply don't have Arial installed). This change has been released in v1.10.2.

Whilst I am still open to implementing your solution, I would like to know what the advantage is of your implementation vs: the tinyicon library I'm using before I do so, and in order to use yours (or a variation of yours) I would also require your project to have a valid LICENSE file in the repo (eg: something like this). Unfortunately just a comment in an issue stating one can use code does not legally make it open source ;-)

@parth391
Copy link

parth391 commented Dec 2, 2023

Still having the same problem

Mailpit Version: 1.10.2
Firefox Version: 120.0.1 (64-bit)
MacOS Version: 14.1.2 (23B92)

Screenshot 2023-12-02 at 11 32 56 AM

@axllent
Copy link
Owner

axllent commented Dec 2, 2023

@parth391 Thanks for the feedback. Can you please open this and confirm whether the favicon appears as it should on the same platform/browser you tested previously? Thanks.

@parth391
Copy link

parth391 commented Dec 2, 2023

@axllent, Link work perfectly on Firefox and Chrome on MacOS.

Chrome:
Screenshot 2023-12-02 at 6 28 19 PM

Firefox:

Screenshot 2023-12-02 at 6 28 50 PM

@federicobond
Copy link
Author

Hi, I can confirm the test link looks good in my computer too.

@axllent
Copy link
Owner

axllent commented Dec 2, 2023

Thanks to you both for the confirmation. I'll continue to work on this in the coming days as there are a few performance issues I need to address (not the fault of the code, but rather the CPU usage caused by the image generation when the favicon is updated often like in the demo). I also need to add a reset function for when there are no unread counts.

@axllent
Copy link
Owner

axllent commented Dec 7, 2023

@federicobond & @parth391 - I have just released v1.10.3 which is using a modified version of @federicobond's script (thank you again!). Please have a test and let me know if this solves the issue on MacOS?

@parth391
Copy link

parth391 commented Dec 7, 2023

@axllent, v1.10.4 fix the issue on MacOS.

Thanks.

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

No branches or pull requests

3 participants