Skip to content
This repository was archived by the owner on Jan 21, 2023. It is now read-only.

feat: new gamedb site design #122

Merged
merged 5 commits into from
Oct 4, 2018

Conversation

FernandoBasso
Copy link
Contributor

@FernandoBasso FernandoBasso commented Sep 23, 2018

NOTE: This is a WIP. @skmp urged me to show what I already had so people could give feedback.

  • two col game grid on the home page on desktop, 1 col on mobile
  • lazy load video thumbnails
  • load youtube iframe video on click
  • two themes: light (default) and dark with toggle button
  • responsive (even thumbnail and video player)

Seems to be working at least reasonably, but it probably has issues and
bugs. I'm open to discussion and suggestions about code and the site
design.

Light version:
gamedb-light-idea01

Darker version:
gamedb-dark-idea01

NOTE: We are discussing the next steps and ideas for the gamedb website here: #125

- two col game grid on the home page on desktop, 1 col on mobile
- lazy load video thumbnails
- load youtube iframe video on click
- two themes: light (default) and dark with toggle button
- responsive (even thumbnail and video player)

Seems to be working at least reasonably, but it probably has issues and
bugs. I'm open to discussion and suggestions about code _and_ the site
design.
.tern-project Outdated
"browser"
],
"loadEagerly": [
"path/to/your/js/**/*.js"

Choose a reason for hiding this comment

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

Isn't this a placeholder that needs to be filled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, not a placeholder. Just an example. This file is not used for the site. It is by editors to know how to intelligently assist us with JavaScript. I guess we can remove it from the project.

Choose a reason for hiding this comment

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

Oh ok. "path/to" is usually placeholder syntax. I just wasn't sure.

.tern-project Outdated
],
"dontLoad": [
"node_modules/**",
"path/to/your/js/**/*.js"

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idem.

@@ -3,7 +3,13 @@
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
{%- seo -%}
<link rel="stylesheet" href="{{ "/assets/main.css" | relative_url }}">
<link rel="stylesheet" href="{{ "/assets/main.css?v=" | append: Time.now.to_i | relative_url }}" />

Choose a reason for hiding this comment

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

Instead of the time, couldn't this use the hash or something related to the revision? It seems excessive to prevent caching it when it won't actually change between commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I have used this locally to help me with dev.

Copy link
Owner

Choose a reason for hiding this comment

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

Technically jekyll templates are evaluated on site build, so the time field should remain constant after the site is deployed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, one option is to use the time. On each build, it would generate a new timestampe that would cache-bust all css and javascript when the user acesses the site, even though many times neither will have changed (ex, we change some text, add a new game, etc), sometimes only one or the other, and the users would still re-download all js and css.

Other option is to set a css and js version variables on _config.yml, use it to cache-bust js and css and bump it only when needed, but then we would need to manually update .config.yml when we change css and/or js. Doing that is similar to manually setting versions like I did so far in my tests: in the place you include the asset. Just that it would be configured in one place.

</script>

{% assign nocache = 'now' | date: '%s' %}
<script src="{{ "/js/scripts.js?v=" | append: nocache | relative_url }}"></script>

Choose a reason for hiding this comment

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

Shouldn't this be using the same method as above (along with the comments about it)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. This was just another approach I was trying.

<img
class="video-thumbnail"
data-src="http://img.youtube.com/vi/{{ game.videos.first.yt }}/0.jpg"
src="{{ "/assets/images/placeholder-image-large.jpg?v=1" | relative_url }}"

Choose a reason for hiding this comment

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

Per the previous comments, you now have a 3rd method for caching. Isn't this going to get hard to track?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, very hard :D Again. I was just trying different approach locally.

I have yet another one in mind: set the css and js 'cache' variable in _config.yml and update it there when needed, and always use it for css/js cache busting.

@AbandonedCart
Copy link

Is there a specific logic behind the column choices over a flexible layout that adjusts to the size of window?

@@ -15,17 +15,10 @@

{%- include footer.html -%}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js" integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>

Choose a reason for hiding this comment

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

https://code.jquery.com/jquery-3.3.1.min.js
Not related to the change, but if you happen to run into issues with the obscure cloudflare version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use another method of loding jQuery, sure.

Choose a reason for hiding this comment

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

You probably don't need to switch, but the "official" link was being posted just in case.

@FernandoBasso
Copy link
Contributor Author

Is there a specific logic behind the column choices over a flexible layout that adjusts to the size of window?

No specific reason other than some personal views.

The site was originally a narrow 800px on desktop (and one column layout for desktop and mobile, default Jekyll layout). I increased it to 960px for the desktop and made it "two columns on desktop", "one column on mobile". It was a small improvement, I dare say (if not an improvement, let us consider it as just another approach then).

I'm not sure about making it as flexible as to use the full viewport width, though. But here I speak my own personal preferences. I generally dislike it when a website fills the screen with 1920px wide (in case of my display) worth of stuff. I find it too much. Or course, that is worse for text. For a grid of games I guess it would not be necessarily "bad".

If we made the home full width, flexible, then we have to make (I think) the game page narrower, perhaps? Although I do not think (as I said) that the game grid on home page could look be OK if it took the full width of the screen, the game page has more chances of not looking good (again, my personal opinion) because there would be no grid (we could make video grids on the game page since some games have two or more videos, but then, not sure it would make sense). Again, I think the full width would be too much in certain displays.

That is my overall take on the flexible, full with approach. Anyways, there are pros and cons every approach we could possible think of. Perhaps my arguments make sense to you, perhaps they don't.

@AbandonedCart
Copy link

It's solid logic, which is all I was asking about. You may want to consider doing mobile, tablet, and desktop with tablet either being a smaller 2 or larger 1.

@skmp
Copy link
Owner

skmp commented Sep 24, 2018

I don't think we need to overcomplicate this. 2 for wide, 1 for less wide sounds just fine.

Use a single way to cache-bust CSS and JavaScript. The main
CSS or the theme-related CSS is changed, we should increment
the respective variables on `_config.yml' so we bust the
cache for those files a new site build is generated.
@FernandoBasso
Copy link
Contributor Author

I think the main, initial part of what I intended to do is done. Let me know if there is something from this set of features/code that you guys think should still be improved.

`tern' is no longer maintained. Let's switch to TSServer
for checking and helping with JavaScript.
Copy link

@AbandonedCart AbandonedCart left a comment

Choose a reason for hiding this comment

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

Looks like you cleaned it up nice and settled on one cache method. 👍

@luserx0 luserx0 self-requested a review October 3, 2018 23:47
Copy link
Contributor

@luserx0 luserx0 left a comment

Choose a reason for hiding this comment

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

Great job extending our previously undecorated theme!

  • Some notes I'd like to add:
    • The dark theme should be something else, this sort of blue interferes with the reicast logo. Something along the grey palette or even a switch between the general background and the per game highlight could do.
    • There should be a search bar for the games included (not everybody knows about CTRL+F).
    • You should update the about section (the already existing footer could make a great scaffold).
    • A little more info on the README, about what you've done, would be great.

Keep up the good work!

@luserx0 luserx0 merged commit dd29350 into skmp:master Oct 4, 2018
@FernandoBasso
Copy link
Contributor Author

FernandoBasso commented Oct 4, 2018

Let's try some stuff about the dark color soon, then, and the search bar would be a very useful feature.

I'll try to work on your suggestions asap. Thanks for the earnest feedback. Much appreciated.

@skmp
Copy link
Owner

skmp commented Oct 4, 2018

Looks really good! @FernandoBasso why don't you create an ticket with a list of ideas on what you want to work on next, and we can discuss there about next steps?

@FernandoBasso
Copy link
Contributor Author

FernandoBasso commented Oct 6, 2018

NOTE: We are discussing the next steps and ideas for the gamedb website here: #125

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

Successfully merging this pull request may close these issues.

4 participants