GH Pages: simplify menu - #1071
Conversation
* Removes the 'Support", "Test" and "Coverage" links. * Removes the "Support" page. * Adds GH icon to the "Contribute" link.
schlessera
left a comment
There was a problem hiding this comment.
As previously discussed, I'm on board with removing these links in the navbar and just pointing towards GH by default. A few things I'd like us to sort out before this goes in:
1. The icon colour fights the navbar
The inlined SVG has a hardcoded fill="#181717", while our navbar is $color-brand: #64A5DE with .navbar { color: #fff } and every single nav link rendering white with a text-shadow. A near-black octocat on that blue is legible, but it reads as a foreign object rather than as part of the menu. I think it should be white, and I verified against GitHub brand guidelines that this is permitted. As shown in the comment, I'd suggest moving away from base64 and allwoing the SVG to inherit the color from CSS.
2. The menu actually gets taller, even though we removed three items
Your own screenshots go from 67px to 77px. That's the <img width="12" height="12"> with vertical-align: text-bottom growing the line box past the line-height: 26px we set on the nav links. Sizing in em and doing the alignment in the stylesheet fixes it. Details in the inline comment.
3. /support/ turns into a hard 404
https://requests.ryanmccue.info/support/ is a live, indexed URL. We don't have a redirect in place, and test-ghpages.yml only runs bundle exec jekyll build, so nothing in CI will ever tell us about the dead link.
The content itself was genuinely stale, so I fully agree it can't stay as-is. But rather than dropping the URL, I'd like us to redirect it. I dug into whether our setup supports that, and it does.
Our site is still a "legacy" GH Pages build (GitHub builds it server-side from the gh-pages branch), which means we're limited to GitHub's whitelisted plugin set, but jekyll-redirect-from is on that whitelist.
It also already ships as a dependency of the github-pages gem we pull in via the Gemfile, so there is nothing to add there either. It just needs to be enabled in _config.yml:
plugins:
- jekyll-github-metadata
+ - jekyll-redirect-from
- jekyll-sitemap
- jemojiThen support/index.md becomes a front-matter-only stub with a redirect_to, see my inline comment on that file for the exact content.
Two caveats worth knowing about, so we don't get surprised:
- Front matter is not run through Liquid, so we can't use
{{ site.github.issues_url }}inredirect_to— the URL has to be hardcoded. - On GH Pages this can only ever be a client-side redirect (meta refresh plus
<link rel="canonical">plus a bit of JS), not a real 301. That's fine for our purposes: the canonical link is what search engines act on, and visitors land where they need to.
Smaller things
alt="GitHub" next to the visible word "Contribute" makes screen readers announce "GitHub Contribute" — see the inline comment.
| <li> | ||
| <a href="{{ site.github.repository_url }}"> | ||
| <img src="data:image/svg+xml;base64,PHN2ZyByb2xlPSJpbWciIHZpZXdCb3g9IjAgMCAyNCAyNCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48dGl0bGU+R2l0SHViPC90aXRsZT48cGF0aCBmaWxsPSIjMTgxNzE3IiBkPSJNMTIgLjI5N2MtNi42MyAwLTEyIDUuMzczLTEyIDEyIDAgNS4zMDMgMy40MzggOS44IDguMjA1IDExLjM4NS42LjExMy44Mi0uMjU4LjgyLS41NzcgMC0uMjg1LS4wMS0xLjA0LS4wMTUtMi4wNC0zLjMzOC43MjQtNC4wNDItMS42MS00LjA0Mi0xLjYxQzQuNDIyIDE4LjA3IDMuNjMzIDE3LjcgMy42MzMgMTcuN2MtMS4wODctLjc0NC4wODQtLjcyOS4wODQtLjcyOSAxLjIwNS4wODQgMS44MzggMS4yMzYgMS44MzggMS4yMzYgMS4wNyAxLjgzNSAyLjgwOSAxLjMwNSAzLjQ5NS45OTguMTA4LS43NzYuNDE3LTEuMzA1Ljc2LTEuNjA1LTIuNjY1LS4zLTUuNDY2LTEuMzMyLTUuNDY2LTUuOTMgMC0xLjMxLjQ2NS0yLjM4IDEuMjM1LTMuMjItLjEzNS0uMzAzLS41NC0xLjUyMy4xMDUtMy4xNzYgMCAwIDEuMDA1LS4zMjIgMy4zIDEuMjMuOTYtLjI2NyAxLjk4LS4zOTkgMy0uNDA1IDEuMDIuMDA2IDIuMDQuMTM4IDMgLjQwNSAyLjI4LTEuNTUyIDMuMjg1LTEuMjMgMy4yODUtMS4yMy42NDUgMS42NTMuMjQgMi44NzMuMTIgMy4xNzYuNzY1Ljg0IDEuMjMgMS45MSAxLjIzIDMuMjIgMCA0LjYxLTIuODA1IDUuNjI1LTUuNDc1IDUuOTIuNDIuMzYuODEgMS4wOTYuODEgMi4yMiAwIDEuNjA2LS4wMTUgMi44OTYtLjAxNSAzLjI4NiAwIC4zMTUuMjEuNjkuODI1LjU3QzIwLjU2NSAyMi4wOTIgMjQgMTcuNTkyIDI0IDEyLjI5N2MwLTYuNjI3LTUuMzczLTEyLTEyLTEyIi8+PC9zdmc+" alt="GitHub" width="12" height="12" style="vertical-align: text-bottom;"/> | ||
| Contribute | ||
| </a> | ||
| </li> |
There was a problem hiding this comment.
Two things at once here.
-
The base64 data URI means the icon's colour is baked into the payload, so it can't follow the menu's
color: inherit. If we inline the SVG markup instead, we getfill="currentColor", which gives us white for free and keeps working if we ever change the brand colour or the hover state. It also drops ~1.4KB of unreviewable base64 from every single page, and it lets us size the thing inemso it stops inflating the navbar height. (I know, I know, I was the one originally mentioning base64 ;) ) -
The
alt="GitHub"right next to the visible label makes assistive tech read out "GitHub Contribute" — the icon is purely decorative here, so it should be hidden from AT instead.
| <li> | |
| <a href="{{ site.github.repository_url }}"> | |
| <img src="data:image/svg+xml;base64,PHN2ZyByb2xlPSJpbWciIHZpZXdCb3g9IjAgMCAyNCAyNCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48dGl0bGU+R2l0SHViPC90aXRsZT48cGF0aCBmaWxsPSIjMTgxNzE3IiBkPSJNMTIgLjI5N2MtNi42MyAwLTEyIDUuMzczLTEyIDEyIDAgNS4zMDMgMy40MzggOS44IDguMjA1IDExLjM4NS42LjExMy44Mi0uMjU4LjgyLS41NzcgMC0uMjg1LS4wMS0xLjA0LS4wMTUtMi4wNC0zLjMzOC43MjQtNC4wNDItMS42MS00LjA0Mi0xLjYxQzQuNDIyIDE4LjA3IDMuNjMzIDE3LjcgMy42MzMgMTcuN2MtMS4wODctLjc0NC4wODQtLjcyOS4wODQtLjcyOSAxLjIwNS4wODQgMS44MzggMS4yMzYgMS44MzggMS4yMzYgMS4wNyAxLjgzNSAyLjgwOSAxLjMwNSAzLjQ5NS45OTguMTA4LS43NzYuNDE3LTEuMzA1Ljc2LTEuNjA1LTIuNjY1LS4zLTUuNDY2LTEuMzMyLTUuNDY2LTUuOTMgMC0xLjMxLjQ2NS0yLjM4IDEuMjM1LTMuMjItLjEzNS0uMzAzLS41NC0xLjUyMy4xMDUtMy4xNzYgMCAwIDEuMDA1LS4zMjIgMy4zIDEuMjMuOTYtLjI2NyAxLjk4LS4zOTkgMy0uNDA1IDEuMDIuMDA2IDIuMDQuMTM4IDMgLjQwNSAyLjI4LTEuNTUyIDMuMjg1LTEuMjMgMy4yODUtMS4yMy42NDUgMS42NTMuMjQgMi44NzMuMTIgMy4xNzYuNzY1Ljg0IDEuMjMgMS45MSAxLjIzIDMuMjIgMCA0LjYxLTIuODA1IDUuNjI1LTUuNDc1IDUuOTIuNDIuMzYuODEgMS4wOTYuODEgMi4yMiAwIDEuNjA2LS4wMTUgMi44OTYtLjAxNSAzLjI4NiAwIC4zMTUuMjEuNjkuODI1LjU3QzIwLjU2NSAyMi4wOTIgMjQgMTcuNTkyIDI0IDEyLjI5N2MwLTYuNjI3LTUuMzczLTEyLTEyLTEyIi8+PC9zdmc+" alt="GitHub" width="12" height="12" style="vertical-align: text-bottom;"/> | |
| Contribute | |
| </a> | |
| </li> | |
| <li> | |
| <a href="{{ site.github.repository_url }}" class="gh-link"> | |
| <svg viewBox="0 0 24 24" aria-hidden="true" focusable="false" xmlns="http://www.w3.org/2000/svg"><path fill="currentColor" d="M12 .297c-6.63 0-12 5.373-12 12 0 5.303 3.438 9.8 8.205 11.385.6.113.82-.258.82-.577 0-.285-.01-1.04-.015-2.04-3.338.724-4.042-1.61-4.042-1.61C4.422 18.07 3.633 17.7 3.633 17.7c-1.087-.744.084-.729.084-.729 1.205.084 1.838 1.236 1.838 1.236 1.07 1.835 2.809 1.305 3.495.998.108-.776.417-1.305.76-1.605-2.665-.3-5.466-1.332-5.466-5.93 0-1.31.465-2.38 1.235-3.22-.135-.303-.54-1.523.105-3.176 0 0 1.005-.322 3.3 1.23.96-.267 1.98-.399 3-.405 1.02.006 2.04.138 3 .405 2.28-1.552 3.285-1.23 3.285-1.23.645 1.653.24 2.873.12 3.176.765.84 1.23 1.91 1.23 3.22 0 4.61-2.805 5.625-5.475 5.92.42.36.81 1.096.81 2.22 0 1.606-.015 2.896-.015 3.286 0 .315.21.69.825.57C20.565 22.092 24 17.592 24 12.297c0-6.627-5.373-12-12-12"/></svg> | |
| Contribute | |
| </a> | |
| </li> |
The inline style is also the only one in this file, and all our other navbar styling lives in _sass/_header.scss. I'd rather we stay consistent and keep it there, especially as that's where the height regression gets fixed too. Paired with the suggestion above, inside the existing .nav > li > a block:
.gh-link svg {
width: 1em;
height: 1em;
vertical-align: -0.125em;
}| --- | ||
| layout: page | ||
| title: Support | ||
| --- | ||
|
|
||
| Getting Support | ||
| =============== | ||
| I'm always happy to answer support questions about Requests. There's a bunch of | ||
| ways to get in contact with me: | ||
|
|
||
| * **Twitter**: Send a tweet with your question to | ||
| [@rmccue](https://twitter.com/rmccue), if you can fit it into a single tweet. | ||
|
|
||
| If you think you've found a problem in Requests, please | ||
| [file an issue on GitHub]({{ site.github.issues_url }}) rather than | ||
| contacting me. |
There was a problem hiding this comment.
See point 3 in my review comment — I'd keep this file rather than delete it (while still removing it from the navbar), so that https://requests.ryanmccue.info/support/ doesn't start 404'ing on us, as it is a public, indexed URL right now. With jekyll-redirect-from enabled in _config.yml, the whole file becomes a front-matter-only stub:
---
title: Support
redirect_to: https://github.com/WordPress/Requests/issues
sitemap: false
---A few notes on that:
- No
layoutneeded — the plugin supplies its own minimal template for redirect pages, and any layout we set would be ignored anyway. - The URL has to be written out in full, as front matter isn't processed by Liquid, so
{{ site.github.issues_url }}would end up in there verbatim. sitemap: falsekeeps the stub out ofsitemap.xml. Note that this is the spelling currentjekyll-sitemapreads — thesitemap: exclude: truewe have in404.mdis an older form which I don't think does anything for us anymore. Separate issue, not for this PR.
I went for the issue tracker as the target, but a valid alternative would be to dend them to the #core-http-api channel in WP Slack. What do you think ?
|
@schlessera That looks like an AI review, not a human review.... ? |
|
@jrfnl That is a combination (letting AI dig into the details, then manually curating/adapting). |
|
@schlessera In that case, you and your AI can fix it up yourselves too. I can't be bothered to explain to some mansplaining machine what motivated certain choices I made. |
Previously looked like this:

Should now look something like this:

If this doesn't look correctly when rendered, some further tweaking might be needed, but we can iterate on that.