-
Notifications
You must be signed in to change notification settings - Fork 335
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
[SPIKE] Update favicons (v5) #4185
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for e9d0dd3 |
This comment was marked as resolved.
This comment was marked as resolved.
3 similar comments
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
cdf4c13
to
0e01376
Compare
0e01376
to
fb294ec
Compare
packages/govuk-frontend/src/govuk/assets/images/govuk-icon-mask.svg
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to chuck away the alpha channel from these PNGs or try limiting to 256 colours not full sRGB
ImageOptim will nearly save 70% on this big one, down to just over 5KB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As these aren't the final assets, I'm not gonna do this just yet, but optimisation is definitely on the cards for later. ✌️
@@ -0,0 +1,19 @@ | |||
{ | |||
"icons": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much do we need to match these up with template.njk?
E.g. We've not got a 48x48 size here but maybe that's fine as it's an *.ico
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think they practically need to match at all. If the only reason we're providing a manifest is for Android devices, we could get away with only defining the 192x192 image.
I included the 180x180 and 512x512 as options in case a requesting service or device might find them useful, as we already have them on hand, and to provide a little bit of futureproofing in case iOS does start using the manifest (as we're seeing with macOS Sonoma), or some other usage comes along that prefers an even larger graphic to be available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…provide a little bit of futureproofing…
Yeah for this bit
i.e. For future proofing, and as a safety net, should we match template.njk to prepare for the day other browsers also ship support for manifest.json?
Making sure all the familiar sizes they support are still in there
I've double checked the What’s New in Safari documentation to find when manifest support was added (Safari 11.1) but they default to apple-touch-icon
in the HTML
Future browsers might default to icons
in the manifest instead 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sizes wise, we're all set for the Google docs for Chromium browsers:
For Chromium, you must provide at least a 192x192 pixel icon, and a 512x512 pixel icon. If only those two icon sizes are provided
But given their examples, we likely want an SVG touch icon too?
Chromium-based browsers also support SVG icons which can be scaled arbitrarily without looking pixelated and that support advanced features like responsiveness to
prefers-color-scheme
, with the caveat that the icons do not update live, but remain in the state they were in at install time.
"icons": [ | |
"icons": [ | |
{ | |
"src": "/images/govuk-icon.svg", | |
"type": "image/svg+xml", | |
"sizes": "512x512" | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add, Apple's documentation has widened since alphagov/govuk_template#300
They suggest 1024×1024 for auto scaling, but for exact pixel sizes they list:
iPhone
- 180×180
@3x
— Home screen - 120×120
@2x
— Home screen
iPad
- 167×167
@1x
— Home screen (iPad Pro) - 152×152
@1x
— Home screen (iPad, iPad mini)
Mac
- 512×512
@1x
— Desktop, Dock - 1024×1024
@2x
— Desktop, Dock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could supply all of those if we really want to, it's just not really necessary.
I've stuck with a single 180x180 that seems to be the sole size used in many modern implementations and recommended by folks dedicated enough to write about the subject.1234
I'm not too concerned with re-defining every single icon from the template in the manifest either. The backwards-compatible nature of the web means that even if Safari, et al. update to prefer the manifest, they're unlikely to only use the manifest, and letting them choose the icon that fits best is why we specify size information in the first place.
Footnotes
-
https://evilmartians.com/chronicles/how-to-favicon-in-2021-six-files-that-fit-most-needs ↩
-
https://dev.to/masakudamatsu/favicon-nightmare-how-to-maintain-sanity-3al7 ↩
-
realfavicongenerator.net changed to only supply a single 180x180 icon way back in 2016 — /~https://github.com/RealFaviconGenerator/realfavicongenerator/issues/211 ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's only one size missing if we say the desktop sizes are out of scope?
- 120×120
@2x
— Home screen
Maybe add the SVG though? #4185 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opted to relent and add all of the icons to the manifest. Seemingly this isn't unexpected (MDN has examples that include even the .ico being defined within the manifest), but I'll be surprised if at least some of these aren't superfluous, being unused by any current browser/device.
I'm not sure whether the current SVG supplied is really suitable for the intended use cases either. It's not the same design as the PNGs offered and doesn't come with any included margins, so if it were to be selected for an app icon on a device it may end up looking a bit weird. Something to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removes apple-touch-icon sizes smaller than 180x180. These aren't used by Apple devices released in the last few years and lower resolution Apple devices can scale down the larger image automatically.
Based on your findings, wondering if the 192px icon is enough to delete this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more of a design preference question. We have 180x180 for iOS because their icons are 60x60 dips, rendered on all recent devices at 3x resolution. We can be confident that a 180x180 image will scale perfectly and remain crisp.
We're providing a 192x192 image for Android for the same reasons, except their icons are rendered at 64x64 dips.
I couldn't see any evidence that either functionally needs the icons to be those sizes—we could toss a 3000x3000 monster at both and be done with it—but supplying the sizes that look the best on each feels like the nicer thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @querkmachine makes total sense to keep it
To double check, I've reviewed the Apple documentation in #4185 (comment) and found a few gaps
Mainly 2018, 2019 devices (iPhone X) at @2x
or the previous non-retina iPad Pro and iPad mini
Seems fair to follow what we did back in 2017 and review them all again
@querkmachine If you give this branch a rebase, we fixed the Rollup build failures in #4208 |
- Updates favicons to use the new Tudor crown. - Removes apple-touch-icon sizes that are no longer in common use. - Adds SVG favicon. - Adds manifest file and icons for Android devices. The assets used in this commit are not final and are subject to change.
We anticipate that some servers may not be configured to correctly serve the .webmanifest content-type, so we're gonna stick with the (seemingly now legacy) .json extension for now.
fb294ec
to
02a9411
Compare
The `maskable` keyword tells Android devices that the icon has been designed with a circular, central safe area, and that Android launchers can safely crop the edges of the icon instead of shrinking the icon down: https://css-tricks.com/maskable-icons-android-adaptive-icons-for-your-pwa/ Despite the name, these are not 'masks' in the same way as the Safari icon. The Safari icon is an alpha mask, which is indicated using the `monochrome` keyword instead.
Closing this spike for now as I think it's accomplished what it needed to. A new PR has been created for the implementation of new icon formats using the Tudor crown: #4354 Depending on the scheduling of the v5 release and GOV.UK comms, we may also have a 'transitional' PR that implements the new formats but continues using the St. Edward's crown (but that doesn't exist yet!) |
Provisionally update the favicons based on my thoughts in this comment (#4175 (comment)), implementing all except for the 'maskable' version of the Android icon, as that would likely require a unique graphical treatment to ensure legibility.
Warning
The graphical assets used in this PR are not final. The PR, whilst in draft, is intended to test changes to what icons we provide and how we implement them from a technical perspective. Final iconography will be produced as part of #4058.
Changes
sizes
attribute where Chrome would always use the ICO even when an SVG favicon was available (sizes
issue detailed here in section 2.3).rel="shortcut icon"
torel="icon"
. Theshortcut
keyword was only needed for supporting Internet Explorer 8 and earlier, which Frontend no longer does.apple-touch-icon
sizes smaller than 180x180. These aren't used by Apple devices released in the last few years and lower resolution Apple devices can scale down the larger image automatically.prefers-color-scheme
CSS query to change the colour of the crown depending on the user's colour settings.manifest.json
file listing all of the available PNG icon sizes.Screenshots
All of the below display the icons from this PR alongside the current favicon that GOV.UK serves to that browser, so that the differences between their appearance can be clearly seen,
Chrome 116, light system theme
SVG favicon.
Chrome 116, dark system theme
SVG favicon.
Edge 116, light system theme
SVG favicon.
Edge 116, dark system theme
SVG favicon.
Firefox 115, light system theme
SVG favicon.
Firefox 115, dark system theme
SVG favicon.
Safari 16.6 on macOS, light system theme with separate tab bar
Safari mask icon.
Safari 16.6 on macOS, dark system theme with separate tab bar
Safari mask icon.
A white background is added to the icon as Safari surmises that the configured mask colour is too dark to be visible against the dark system theme.
Safari 16.6 on macOS, compact tab bar
Safari mask icon.
When Safari is configured to use the compact tab bar layout, the tab bar takes on a colour from the current page (either automatically, or the one defined by
theme-color
meta tag).As there is no guarantee that the favicon will have contrast against those colours, a white background is added around it.
Internet Explorer 11
ICO favicon. Captured through Browserstack, which may have distorted the appearance of the icons due to video compression.
Safari 17.0 (beta) on iOS, tab switcher
ICO favicon.
iOS 17.0 (beta) home screen
Apple touch icons.
macOS 14.0 (beta) dock
Uses the largest icon defined in the manifest file, if any are defined. If not, falls back to using
apple-touch-icon
. If neither are available, the favicon is used. If none of them are available, macOS generates an icon using the first letter of the page's title with a background colour.Android 13
Android 192 PNG favicon.
Both the homescreen and Android version of Chrome use the Android icon defined in the manifest. I couldn't get screenshots of this because the device security policy disallowed it, but they work.
The
theme-color
HTML attribute still works too, and has not been affected by the addition of a manifest file.