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

Use favicon #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use favicon #3

wants to merge 1 commit into from

Conversation

jakoblorz
Copy link

@jakoblorz jakoblorz commented Jun 5, 2018

Warning: Opinionated Pull Request

It is unclear if the favicon was dropped on purpose, but favicons improve recognition across devices. Missing out the favicon does not prevent the browser to display something: here on my personal rig, chrome on linux shows a empty rectangle / page icon instead. Combined with the title "👋 Hello, GitHub ", it looks like something is broken.

crop

This is why I took the freedom to (in my opinion) improve the page:

  1. Added the waving hand emoji from twitter's open source emoji collection as favion.png;
    emojis are subject to CC-BY 4.0, check compliance
  2. Added the favicon into the default.html layout
  3. Altered order of the header to ensure that the tab will not show two emojis next to each other
    • Before: 👋 Hello, GitHub
    • After: Hello, GitHub 👋

crop2

If you want to affiliate this hello blog directly with GitHub, you could also use GitHub's Octocat favicon:
octocat

I wish you a good hand with GitHub @natfriedman

@armudgal
Copy link

armudgal commented Jun 5, 2018

All the best @natfriedman :)

@levrik
Copy link

levrik commented Jun 5, 2018

@jakoblorz I think that would look weird on Windows to keep the emoji in the title since it's displayed with colors here.
image

@jakoblorz
Copy link
Author

jakoblorz commented Jun 5, 2018

@levrik yeah, I am aware of that. The problem is that removing the title emoji completely would require the 👋 emoji to be removed from the post's title. The post's title is reused as the title of the whole page (which makes sense from a frontend hierarchy perspective). I tried to bridge this problem by moving the emoji in the title.
Maybe someone can find a workaround for that - some kind of Gem which removes emojis from jekyll posts before adding them as the page's title.

What I already tried is to remove the emoji and integrate the favicon.png in the posts markup:
[wave]({{ "/favicon.png" | absolute_url }}) Hello, GitHub
crop3

That solves the problem but obiously brings the title emoji completely out of proportion.
crop4

@amin007
Copy link

amin007 commented Jun 7, 2018

wow amazing

@dostarora97
Copy link

LGTM 👍

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

Successfully merging this pull request may close these issues.

5 participants