xqemu / xqemu.com

Sources for the XQEMU website (xqemu.com)
https://xqemu.com
6 stars 12 forks source link

Hide github icon unless on developer page #41

Closed DiscoStarslayer closed 4 years ago

DiscoStarslayer commented 5 years ago

Closes #39

Option 1 (this pr)

Overrides to the existing partials so that the github icon is not shown on any pages other than the developer pages.

I like that this implementation keeps the conditional display of the github icon in the templates, statically rendering to html.

I don't like how much code needs to be repeated:

Option 2

I also have an alternative implementation done with JS + CSS: https://github.com/DiscoStarslayer/xqemu.com/pull/1/files

I feel the intent of the code is clearer on the JS implementation, but I am not a big fan of the fact that it has to run in the browser instead of just being a statically rendered element.

Option 3

Just remove this icon all-together to avoid the added complexity. One line of CSS + a config change to mkdocs.yml will do this pretty cleanly.

JayFoxRox commented 5 years ago

I vote to remove the GitHub logo from the theme altogether.

On IRC, there have been concerns about us removing the GitHub logo, but this change is mostly to avoid users coming to GitHub, thinking it's a forum. We'll still prominently feature our open-source background; probably even bigger than the current logo.

I believe we should prominently display it on the "Welcome Developers" page (GitHub logo, repo link and CI status). Also see #43 .

DiscoStarslayer commented 5 years ago

I agree with removing entirely, as it reduces complexity of the implementation somewhat.

As for implementation direction:

  1. should we go with overriding the partials (adding two HTML files to a overrides directory, like done in this PR)
  2. Hiding the github element with CSS

1 is cleaner from the user perspective, as we don't ship them un-used HTML, but less clean from a developer position as we need to override the theme partials and it's not immediately clear why from just looking at the code.

2 is much cleaner from a code perspective, as it's a single line change, and obvious what it is doing. But we are shipping HTML to the user that they will never see so it feels kinda klunky.

There is also a third option, looking at the existing partials, it seems to suggest that simply removing the repo_url config will remove the github links. In practice, this does not work, as removing the config leaves a bunch of white space where the github links used to be:

Screen Shot 2019-05-30 at 9 41 31 AM Screen Shot 2019-05-30 at 9 41 40 AM

Maybe it's worth trying to upstream a PR to fix this bug in the theme so we can keep our own codebase clean?

JayFoxRox commented 5 years ago

Yes. Removing repo_url should work (I thought it worked fine in the past?). It's also my preferred method. Sounds like an upstream issue.

GXTX commented 4 years ago

I don't think this is a good idea. I often find myself quickly viewing a projects website for the sole purpose of looking for their Github.

DiscoStarslayer commented 4 years ago

Fair comment.

I think a counter to your point is this doesn't remove the github link entirely. The developer section has a link to the github repo prominently on the page, it also details important context for any potential contributor (XboxDev group ect.)

I think this page is more valuable than the github repo by itself, so this change will help reduce non-developer users from hitting the github repo (poor experience, multiple screen pages to readme, readme not the most descriptive/useful for user). I also think this will improve the experience for potential contributors interested in the project as they are provided context before being routed to the repo which is pretty barren in this regard.

JayFoxRox commented 4 years ago

What shall we do with this?

JayFoxRox commented 4 years ago

I'll close this now. It's currently not going anywhere. Please create a new PR if this is being worked on again.

Closed.