Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Add favicon to project #49

Merged
merged 2 commits into from
Dec 3, 2018
Merged

Add favicon to project #49

merged 2 commits into from
Dec 3, 2018

Conversation

tbille
Copy link
Contributor

@tbille tbille commented Nov 30, 2018

Add favicon to project.

Fixes #11

Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@anthonydillon
Copy link
Contributor

Can you do me a favour and add <meta name="viewport" content="width=device-width, initial-scale=1" /> in there too, plz!

@anthonydillon
Copy link
Contributor

Awesome thanks 👍

@tbille tbille merged commit c5bf549 into canonical:master Dec 3, 2018
@tbille tbille deleted the add-favicon branch December 3, 2018 16:26
@nottrobin
Copy link
Contributor

I was gonna add a comment on this, then it got merged... I'm gonna comment anyway:

@anthonydillon do we have a standard for how to add favicons? There's a whole spectrum of options from just one image to about 20, including a bunch of "apple-touch-icon"s and "og:image".

I'd prefer simplicity personally. I'm not sure why we would need a 32 pixel one and a 16px one. Personally I think a single link to a 32 or 48 pixel icon should do just fine. But do you have another standard for a reason?

@anthonydillon
Copy link
Contributor

Happy to restrain from adding og or Apple favicons as this site is not to be shared. The two sizes are for single DPI screens and double DPI screens.

@nottrobin
Copy link
Contributor

nottrobin commented Dec 4, 2018

My thinking was that every modern browser will happily display a 32x32 favicon, and it should look fine for both cases, so why complicate things with 16x16? I don't think the difference in size between 32x32 and 16x16 is going to make any difference at all to performance.

@anthonydillon
Copy link
Contributor

anthonydillon commented Dec 4, 2018 via email

@nottrobin
Copy link
Contributor

@anthonydillon from that page:

Site Icons in Internet Explorer 9

I think I win the argument.

@nottrobin
Copy link
Contributor

@anthonydillon more seriously, I'd rather not be a team that just does stuff because "Microsoft recommends it". When people have good justifications for their advice, great. We can read the justification and decide it totally makes sense and we agree.

In this case, Microsoft are also recommending that we create icons in .ico format. Their advice is significantly out-of-date - PNG icon support is now ubiquitous, and IE9 has disappeared into the mists of history.

And they don't actually recommend that you use all 3, they instead say:

your icons should support the following image sizes:
Recommended | 16x16, 32x32, 48x48

I would argue that a single 48 pixel icon is "supporting" all of those image sizes.

So I don't understand, in this day and age, why we can't just include 1 icon in whatever the largest size we need is and have done with it. Given that, for an icon that is 48 pixels wide, the time taken to do the HTTP handshake will exceed the time taken to download the image in 98% of cases, we really don't need to worry about providing 3 separate icons.

Do you have an actual counter-argument to this point?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants