pages-themes / hacker

Hacker is a Jekyll theme for GitHub Pages
https://pages-themes.github.io/hacker/
Creative Commons Zero v1.0 Universal
957 stars 1.17k forks source link

[Feature] Set dark background colour for search and status bar on mobile devices #67

Closed olifre closed 3 years ago

olifre commented 3 years ago

Mobile devices using a bright theme visiting a hacker-themed page do not adapt their header / status bar colour to the dark background colour of the hacker theme.

This is possible with the following header tags:

<!-- Chrome, Firefox OS and Opera -->
<meta name="theme-color" content="#151515" />
<!-- Windows Phone -->
<meta name="msapplication-navbutton-color" content="#151515" />
<!-- iOS Safari -->
<meta name="apple-mobile-web-app-status-bar-style" content="#151515" />

(see also: https://stackoverflow.com/questions/26960703/how-to-change-the-color-of-header-bar-and-address-bar-in-newest-chrome-version-o for screenshots and examples)

davorpa commented 3 years ago

Hey @olifre

Nice enhacement.

Since document is HTML5 doctyped, close meta tags are not needed. To maintain same code guidelines on entire template I recommend remove the trailing slash.

The color #151515 matches with defined for body background... https://github.com/pages-themes/hacker/blob/7b7303abefd44daf037249f1f9281d46ee1e1a91/_sass/_default_colors.scss#L5 https://github.com/pages-themes/hacker/blob/7b7303abefd44daf037249f1f9281d46ee1e1a91/_sass/jekyll-theme-hacker.scss#L4 ... but according to Safari guidelines, an extra meta tag is needed to make the color effective apple-mobile-web-app-capable (What implies enable it?). Moreover, the color can only take three values: default, black and black-translucent. With this last, body background color is used to compute status bar color but with texts in white and page content overlaps it (20px). For this theme, it could serve black value, it isn't?

Then the snippet looks as some like this:

<!-- Chrome, Firefox OS and Opera -->
<meta name="theme-color" content="#151515">
<!-- Windows Phone -->
<meta name="msapplication-navbutton-color" content="#151515">
<!-- iOS Safari -->
<meta name="apple-mobile-web-app-capable" content="yes">
<meta name="apple-mobile-web-app-status-bar-style" content="black-translucent">

What do you think about?

olifre commented 3 years ago

What do you think about?

That looks like several great improvements, thanks for this research :+1: ! Did you already test this with a Safari mobile browser? I don't have access to one, but a test might be worthwhile to check if there are also unintended side-effects from the "capable" meta tag.

davorpa commented 3 years ago

Did you already test this with a Safari mobile browser?

Sorry, me neither, no iPhone 😣

I've been taking a look at some webs, also the other themes and in Cayman was approved with black-translucent and not -capable https://github.com/pages-themes/cayman/pull/43/commits/fa9637941afa21f1e5b186b894ba83e890b54e9c Screenshot_20210629-215258

There are enough top gap how to allow overlap mode, so black-translucent it's fine. With black it would also look good due to this theme has a quite dark background color. Screenshot_20210629-215216

So... both are valid for me.

<!-- Chrome, Firefox OS and Opera -->
<meta name="theme-color" content="#151515">
<!-- Windows Phone -->
<meta name="msapplication-navbutton-color" content="#151515">
<!-- iOS Safari -->
<meta name="apple-mobile-web-app-status-bar-style" content="black-translucent">
<!-- Chrome, Firefox OS and Opera -->
<meta name="theme-color" content="#151515">
<!-- Windows Phone -->
<meta name="msapplication-navbutton-color" content="#151515">
<!-- iOS Safari -->
<meta name="apple-mobile-web-app-status-bar-style" content="black">

Fix it as you wish 😉. I review your PR if I can 🤗.

olifre commented 3 years ago

I think I'd also expect black-translucent to work fine, and not specifying -capable seems more safe to me. While this does not match the Safari documentation, the screenshot you found seems to show it works nevertheless, and it seems more safe since it does not change other behaviour :wink:. I can also only suggest here, we'll have to wait for someone with commit privileges to review and merge.

davorpa commented 3 years ago

Yep.

The screenshots are captured using a Samsung S5 on Chrome.

I hope that any admin approve and release. It seems that theme projects are a bit abandoned.

Meanwhile there will be to wait for Travis CI build