Require only 1 image for royal coat of arms #3873
Conversation
colinrotherham
added
awaiting triage
| background-image: govuk-image-url("govuk-crest.png"); | ||
| @include govuk-device-pixel-ratio { |
There was a problem hiding this comment.
I've had a reminder from @36degrees that other service teams likely use govuk-device-pixel-ratio() so we'll need to stick with our Deprecating a Sass function or mixin notes to avoid a surprise breaking change
But love that we don't need this now 😊
To help manage that change we can either:
- Keep the mixin but add
@deprecatedwith a CHANGELOG entry in this PR - Keep the mixin but write up a separate GitHub issue to deprecate it in future
There was a problem hiding this comment.
Happy to deprecate this prior to later removal. Given impending v5.0 release, is there an opportunity to deprecate in a prior 4.x release, and remove in v5.0?
If so, I can create a separate issue to remove this mixin and attach it to the 5.0 milestone.
There was a problem hiding this comment.
We've done a "last" (v4.6) release already and next "last last" (v4.7) we'd like to freeze by tomorrow at 5pm. But if you're up for it we can try and take a quick deprecation-only PR against support/4.x branch?
It's a bit of a stretch as time's against us but we massively appreciate the contribution
Alternatively we can deprecate in v5.0 and remove in v5.1 instead?
There was a problem hiding this comment.
As an example, you can see how we deprecated govuk-not-ie8() on support/4.x but added new private versions too. Means we could continue using them (internally) until ready to remove entirely:
There was a problem hiding this comment.
Challenge accepted! #3877
|
Thanks @paulrobertlloyd 🙌 Love a good opportunity to remove things Would you mind splitting out the Allow Crown copyright notice to be removed side of things into another PR? Just to let us consider anything copyright/legal related separately I've leave the Design System squad to have a closer look |
paulrobertlloyd
changed the title
As background-size is supported by IE9, we no longer need to provide a different crest image based on device pixels
Split into a separate PR, #3876 |
|
To be a little contrarian, I think there's still value in us having 1x and 2x versions of the graphic. The different scales exist for more than just legacy IE support—although that certainly helped—but to account for screens of different pixel densities. Although 2x (or even 3x and 4x) pixel densities are common on mid- and high-end smartphones and laptops nowadays, the 1x density is still pretty common, especially on desktop PCs. Even the not-particularly-cheap, not-particularly-old external monitor I use at home is low enough pixel density to still use the 1x image. If we remove the 1x image, we're forcing all devices to download and use the larger image, even if there is no benefit to it doing so. Even though this is only a few kilobytes per load (caching notwithstanding), that feels like it would build up to a lot of unnecessary data usage quite quickly. (Unfortunately device pixel density isn't something GOV.UK collects as part of its analytics, so it's only possible to estimate based on generalisations.) Personally, I don't think this change serves a need for users, for the environment, or for the data bills of our servers and users. |
|
As a bit of related info, we could still remove Autoprefixer handles Input@media (min-resolution: 2dppx) {}Autoprefixer defaults@media (-webkit-min-device-pixel-ratio: 2),
(-o-min-device-pixel-ratio: 2/1),
(min-resolution: 2dppx) {}Autoprefixer using
|
|
Hi @paulrobertlloyd, I'm going to close this PR. There are whispers coming through the grapevine that the Royal Arms here might be receiving an update soon(ish, maybe), so we might revisit this work at that time. Sorry for having this sit around for so long just to say 'not yet'! |
|
@Beeps Makes sense, and thanks for the update. Quietly hoping the revised coat of arms are simplified to the extent that they can be saved as an SVG file that’s smaller than a PNG. Probably not, but that’d solve a whole number of issues! 🤞 |
Currently there are 2 versions of the royal coat of arms for single and double pixel density screens, the different images served based on a resolution media query. My belief is that this implementation was chosen as IE8 didn’t support the
background-sizeproperty in CSS.With IE8 support being dropped, it’s now possible to have a 2x image, and use
background-sizeto scale it down by half.As this is the last place the
govuk-device-pixel-ratio()mixin is used, I have deprecated this helper.I’ve also further optimised the large
govuk-crest.pngimage. Currently it is 8,884 bytes, the version included in this PR is8,3657,624 bytes.Sidenote: I tried using an SVG, but given the complexity of the image, this results in a much larger file size.