-
-
Notifications
You must be signed in to change notification settings - Fork 93
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 improvements #664
favicon improvements #664
Conversation
1. no more advertising favicon.svg as icon, so clients will ask for favicon.ico on their own 2. favicon.ico manually re-drawn for better legibility (and slightly smaller code size) 3. bug fix: favicon.ico was delivered to clients w/o its last byte (which lead Chromium based browser to not use/show it) Trigger for this change: when using a dark desktop theme, as I do, the black favicon.svg with its transparent background was hardly visible on a dark gray browser tab. Favicon,ico does have a white background, making the black BSBLAN visible on light and dark browser tabs. It is also much more readable than favicon.svg scaled down to 16x16 pixels by a browser. Change tested using - Firefox, Chrome, Chromium on Ubuntu - Safari, Chrome on MacOS - Edge on Win11
Without any screenshots before/after, I'm obviously not going to accept this. Also, if the logo has been changed from "white" to "black" (that's how it reads), then it's also a no-go. |
Obviously! :) I do have before(=left)/after(=right) screenshots for Firefox, using Ubuntu's dark/light desktop scheme, with the browser tabs active/inactive: The original favicon.ico looked like this (scaled to 500 % for clarity here): The proposed new one: Actually, favicon.ico should also contain images at 32x32 and 48x48 resolution, but I would not waste that much code size in a micro controller web server. On all systems I've tested, I consider the changes in this pr here an improvement. However, when scaled up, the *.ico will look worse than the *.svg; I would hope that for such cases (e.g. Apple's Retina displays?) browsers would ask for a favicon.svg, just like they ask for favicon.ico for their tabs without being told so explicitely. Should you decide to keep favicon.svg advertised in the html served by bsb-lan, I would still fix the bug in *.ino and change the *.ico - or completely remove the favicon.ico from html_strings.h and the related code from *.ino https://github.com/audreyfeldroy/favicon-cheat-sheet provides useful info on favicons, btw. |
Ok, thanks, that lookd better indeed! However, the Favicon is also used (at least on iPhone etc.) as the "app symbol" when you add a website as a "shortcut" on the phone's home screen. Maybe Android does the same? In that case, I guess more users would be affected daily by a lower resolution .ico compared to a badly scaled down icon in the browser. I'm not sure what would be necessary to cover both cases (which I'd prefer), and if it's a few bytes more memory, I'd say go with it if we can cover both use-cases then. But if we can have either one only, then I guess we stay with the current version. Do you have any means to test this out by any chance? |
There's no iPhone in my household, and Android 10 on my mobile phone uses generic icons for web site links on the home screen. Could you test this for iPhone? |
I've now noticed that in Chrome on Android 10, I see a favicon for bsb-lan when looking at a tabs overview, while I don't see one with my BSBmonCR, which only serves a favicon.ico and doesn't have a favicon.svg. This could mean that the browser has asked for favicon.svg on its own, or that it still uses the cached icon from bsb-lan before the change described in this pr. I can look into this later today. P.S. It uses favicon.ico, and the reason the icon didn't show for my BSBmonCR is that I had loaded it through a local *.html 8o| |
I have an iPhone, but I'm not near my home so, making these changes and testing them will take time... |
An alternative would be to change the favicon.svg to something that scales well to 16x16 and has a non-transparent background, to be "readable" on all background colors (which differ with the desktop/display theme selected by the user). By most UI guidelines I know, the BSB-LAN logo has too much information/detail to be used as an icon, and having text in icons is also not recommended afaik. (https://developer.apple.com/design/human-interface-guidelines/icons and pages linked from there show one vendor's ideas about icons.) The logo, as used at the top of BSB-LAN's web pages and as a watermark for the /DG plot, could stay the same, of course. A quick new icon could be just the A/flame from BSB-LAN's logo, which would scale well to 16x16 (shown here at 500 % for clarity): |
...or one could try to do it "right" by following https://medium.com/swlh/are-you-using-svg-favicons-yet-a-guide-for-modern-browsers-836a6aace3df e.g. :) |
I think that's too much of a hassle for a rather minor change. My main issue remains the shortcut on the (iOS) home screen which with the current logo makes it clear that it's BSB-LAN. |
...or, as a compromise that mostly addresses what lead me to this change: Make the favicon.svg background white instead of transparent, so that it is better visible on a dark gray browser tab, and keep advertising it as the icon to use. Even with this solution, I would still fix the bug in serving /favicon.ico, and also replace the favicon.ico with my optimized version - or remove everything favicon.ico related from the code? Do you see any problems with favicon.svg having a white instead of a transparent background? |
The only problem I see here is that I won't have the time to make any of these changes. If you want the SVG to have a white background, then go ahead, I'm fine with it. And sure, the bug should be fixed. |
I've now changed the code back to advertising favicon.svg, but made its background white instead of transparent. |
Thanks! Merged. |
I think we're good for now, but thanks :)! |
Trigger for this change: when using a dark desktop theme, as I do, the black favicon.svg with its transparent background was hardly visible on a dark gray browser tab. Favicon,ico does have a white background, making the black BSBLAN visible on light and dark browser tabs. It is also much more readable than favicon.svg scaled down to 16x16 pixels by a browser.
Change tested using