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

[WIP] Dark theme #372

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

aghArdeshir
Copy link

@aghArdeshir aghArdeshir commented Oct 5, 2022

Hi. This PR:

  • adds a small button on top of every page. This small button is responsible for toggling between dark and light themes.
  • persists the user's current choice in the local storage, so the latest selection is persistent across page refreshes and navigations.
  • implements a (n almost) well-looking dark theme.
  • fixes two minor bugs in script.js.
  • reads changes from the user's preferences to enable dark or light themes based on the user's system preferences.

TODO:

  • Improve the structure of the asset/dark-styles.scss file to use meaningful variables and better order of styles.
  • Maybe better icons for light and dark themes.

ONLINE DEMO

https://aghardeshir.github.io/game-programming-patterns/html/command.html

VIDEO DEMO

dark-theme-for-game-programming-patterns.webm

VIDEO DEMO FOR READING THEME FROM SYSTEM SETTINGS

dark-theme-from-system.webm

@@ -31,12 +32,33 @@ function refreshAsides() {

// Find the span the aside should be anchored next to.
var name = aside.attr("name");
var span = $("span[name='" + name + "']");
var span = $("span[name='" + name.replace("'", "\\'") + "']");
Copy link
Author

Choose a reason for hiding this comment

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

There was a case in which the value of name was won't, and this whole selector would have become span[name='won't'], which is a syntax error. This change tries to prevent such cases.

Comment on lines +41 to +43
if (span.position()) {
aside.offset({top: span.position().top - 3});
}
Copy link
Author

Choose a reason for hiding this comment

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

There were cases where span.position() returned undefined. I didn't investigate it deeper, I just patched it this way, so it does not happen anymore.

@aghArdeshir aghArdeshir marked this pull request as draft October 5, 2022 07:20
@aghArdeshir aghArdeshir changed the title Dark theme [WIP] Dark theme Oct 5, 2022
@aghArdeshir
Copy link
Author

@munificent? You explicitly mentioned you won't be actively maintaining this repository anymore. But I thought of mentioning you, as I believe this PR is a must-have and helps in reading the book easier.

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.

1 participant