mekarpeles commented 3 years ago

Originally proposed by @vidhi-mody in #3259 -- this is a community requested feature which is not on the 2021 staff roadmap, but we imagine folks will be happy about this change, so we're happy to guide the community to implement this feature (lower priority)

BrittanyBunk commented 3 years ago

What can anyone do? I think I might've brought this up too somewhere. Mind posting the 2021 staff roadmap somewhere, so people don't waste their time on lower priorities?

Philippos01 commented 2 years ago

Hello, me and my collegue @dbouris are interested in this issue. We can try to apply the css and the javascript needed, in order to make a full functional dark mode. Is it posssible to assign this issue to us?

DarrellRoberts commented 6 months ago

hey @mekarpeles and @jimchamp! I'd love to pick this up if it's still pending if you'd like to assign me?

jimchamp commented 6 months ago

I'm not even sure that there's enough information here to even get started with this. At a minimum (and to start with), we'll want some mock-ups for dark mode views (including the dark mode toggle, which is commonly a moon or sun icon).

DarrellRoberts commented 6 months ago

Hi @jimchamp, that's a very good point & makes complete sense. For the initial mock-ups, is that more a job for the designers?

mekarpeles commented 6 months ago

Thank you @jimchamp and @DarrellRoberts. I think for someone who wants to take this challenge on, commenting on this issue with a minimum plan would be useful, e.g.:

Where does a patron go to set Dark Mode? Likely a global toggle on the website header or footer (or their account settings)
What is the minimal set of changes that should occur in dark mode? (e.g. backgrounds and primary font colors?

Mock-ups would also be useful. It doesn't have to be perfect, even demonstrating how much work this project might take would be a valuable learning.

DarrellRoberts commented 6 months ago

thank you @mekarpeles ! I can work on the minimum plan answering your bullet points, along with screenshots, if that sounds good?

I've created dark mode for other projects before though only on JavaScript frameworks but up for a challenge.

p.s. apologies for missing today's call. I got my timezones mixed up.

DarrellRoberts commented 5 months ago

hey @jimchamp and @mekarpeles, see below for my proposed action plan for implementing dark mode.

In a nutshell, my thinking was that we have three CSS Less mixins: lightmode(), darkmode() and toggletheme(). The toggletheme() mixin is then used for the background colors and other colors for the body element along with its children. A data-theme is added to the body element with JavaScript, telling the toggle-theme mixin whether it is "light" or "dark". The button then allows the user to change this state. I used localstorage for watching this state but I'm open to suggestions if you think there's a better way.

Please note, at this stage I'm not sure where the dark mode button should go nor what the design should look like. Therefore, at this stage, for the dark mode button I've just used a simple HTML button.

Nevertheless, I've provided screenshots at the end of a possible design for the dark mode (used Chrome's automatic dark mode dev tools), though we may want a more desaturated look, but maybe this is more of a job for the designers?

Let me know what you think.

Dark Mode Implementation

CSS

In the static/css/less/mixins.less file, create two mixins: e.g. (line 15)
```
.light-theme()
```

.dark-theme()


.light-theme() will take the default values.

2. Wrap the base value colours within the .light-theme() mixin using the "&" operator
e.g.

.light-theme() { background-color: @beige; & li { color: @dark-grey; } & h1, h2, h3, h4, h5, h6 { color: @grey; } ... }


3. Research the dark mode equivalent for the color variables in colors. Ensure that whichever color you use it corresponds to the AA/AAA guides with the color and background-color properties (you can use websites such as: [Siege Media's Contrast Ratio](https://www.siegemedia.com/contrast-ratio)).
You can also use the automatic dark mode in the developer tools as a guide.

4. When satisfied, add the dark mode colors in the _**static/css/less/colors.less**_ file
e.g.

// beige ... @beigeDM #....


5. Return to the _**static/css/less/mixins.less**_ file and complete the .dark-theme() mixin so that it mirrors the .light-theme() mixin.
e.g.

.dark-theme() { background-color: @beigeDM; & li { color: @dark-greyDM; } & h1, h2, h3, h4, h5, h6 { color: @greyDM; } ... }


6. Create a third mixin called .toggle-theme(). This will be the mixin that listens to the data-theme attribute.
Complete this mixin with the following properties:
e.g. (line 38)

.toggle-theme() { &[data-theme="light"] { .light-theme(); } &[data-theme="dark"] { .dark-theme(); } }


7. In the **_static/css/base/common.less_** import the mixins 
e.g.
`@import "../less/mixins.less";`

8. In the same file, for every element, replace the color, background-color and any other color properites (provided they have been accounted for in the .lightTheme() and .darkTheme() mixins, with the .toggle-theme()) 

e.g. (line 39)

body { font-size: 100%; line-height: normal; .toggle-theme(); font-family: @lucida_sans_serif-1; }


### HTML
9. Add the dark mode button within **_openlibrary/plugins/templates/lib/nav_head.html._** Make sure to add the id, "darkMode".
e.g. (line 129) 
`<button id="darkMode">Dark Mode</button>`

### JavaScript
10. Add the JavaScript logic. There's a useful guide for this by Jaye R.: [Adding New Javascript Files to HTML Templates](https://github.com/internetarchive/openlibrary/wiki/Frontend-Guide), I followed this.

I added two functions to the index.js because I want to first initialise the theme as well as toggle the theme.

This is what I added to the **_openlibrary/plugins/openlibrary/js/index.js_** file
e.g. (line 550)

//theme const bodyTheme = document.getElementsByTagName('body'); if (bodyTheme) { import('./bodytheme') .then(module => { if (bodyTheme) { module.initBodyTheme() } }) }

(line 561)

//Dark Mode const darkMode = document.getElementById('darkMode') darkMode.addEventListener('click', function() { if (darkMode) { import('./darkmode') .then(module => { if (darkMode) { module.initDarkMode() } }) } })


Line 550 binds the initial theme to the body element. 

Line 561) adds a click event listener to the toggle button

11. Then I created a **_bodyTheme.js_** file
e.g. (line 1)

export function initBodyTheme() { const theme = localStorage.getItem('theme') ? localStorage.getItem('theme') : 'light'; const body = document.querySelector('body'); body.setAttribute('data-theme', theme); }


This states then upon loading the page, the data-theme will be taken from the local storage, which is where it is stored. If there is no theme, it reverts to the default: 'light'. By doing this, I allow the theme to remain dark on a page refresh or a new page load. 

12. Then I created the toggle logic in the darkmode.js file:
e.g. (line 1)

export function initDarkMode() { const body = document.querySelector('body'); if (body.dataset.theme === 'light') { body.dataset.theme= 'dark'; } else { body.dataset.theme = 'light'; } }



Following on from the local state, the toggle changes both the data-theme within the body element as well as in the local storage. As the mixins are conditional and listen for the data-theme, changing the data-theme affects the body element and all of its children. 

And that's it.

## Screenshots
For the button I just used a simple HTML element and positioned it next to the logo. This isn't what I'm proposing in terms of the button's design or position and it was just easier to test.

As for the actual design of the button, I would usually use a moon/sunshine icon. For its position, I was thinking the top-right in the navbar but I'm completely open to feedback. 

1. Homepage
![image](https://github.com/internetarchive/openlibrary/assets/139546996/1a562e5d-3795-4432-aaac-bea5a40b8c8d)

2. Book Search results
![image](https://github.com/internetarchive/openlibrary/assets/139546996/6e5afcfa-d1e1-4651-bde3-1e4216bc746b)

3. Single Book page
![image](https://github.com/internetarchive/openlibrary/assets/139546996/49993ceb-1e23-46e4-b3c0-d0559e262ece)

jimchamp commented 5 months ago

Sorry for the delay, @DarrellRoberts, and thanks for putting this together.

I don't think that we need to use a data attribute for this. Just add a new class to the body or html instead.

I'm not sure that I understand the purpose of the toggletheme() mixin. I would expect to have some type of body.dark or html.dark rule that will have the styles for dark mode, but maybe I'm missing something?

Storing the dark mode settings exclusively in localStorage may cause some issues. I believe that the page will already be painted by the time initBodyTheme is called. This means that the page may load with the undesired theme, then suddenly switch to the expected theme. The HTML should be served with the dark class, if dark mode is selected. It will probably be better to use a cookie for this setting. We can check for the dark mode cookie in the /site/body.html template and add the dark class there, if necessary.

We'll want to get some feedback from the design community on the following:

Dark mode color palette
Location and styling of dark mode toggle
Dark mode static assets (like the logo, which doesn't look great in the pics that you provided)

In the meantime, it would probably be useful if a small proof of concept was created for this feature. Maybe something that only toggles the background color of the site from beige to something darker when dark mode is selected on the settings page? I just want to make sure that this can be implemented without a jarring theme change on page load.

DarrellRoberts commented 5 months ago

Hi @jimchamp , thanks so much for your feedback!

Regarding your points:

Add a new class to the body or html rather than use data-theme.

Sure thing, that makes better sense.

Purpose of toggleTheme()

Originally I thought I needed a condition and the toggleTheme() mixin was a way of switching between the two styles. However, after revisiting it, you're right that we just need a .dark CSS class and I was overcomplicating it.

Store darkMode state in cookies rather than localstorage

Sure thing, although I'm having difficulty retrieving the cookie value in the /site/body.html.

I could either set the cookie via JavaScript DOM or in Python in the plugins/openlibrary/home.py file (please say if this is the wrong file to do it).

e.g.

        darkMode = False
        web.setcookie('darkMode', str(darkMode), path='/')

However, when it comes to retrieving the value, I'm not sure where/how to do it and how I can pass it to the /site/body.html template (where, once retrieving the value, I could conditionally append() a 'dark-theme' to the $bodyclass).

I believe this is the method: https://webpy.org/cookbook/cookies, but I'm not sure how nor where to execute it. Would you be able to give some direction?

Feedback from the design community

Sure thing, I'll post a message and link to this issue

Proof of concept on the settings page

Sure thing, once I figure out how to retrieve the cookie value on the /site/body.html, I can submit a PR. Everything else works fine.

jimchamp commented 5 months ago

[...] although I'm having difficulty retrieving the cookie value in the /site/body.html.

Sorry about that! I thought that cookies were exposed to template files, but they are not. Adding something like the following to openlibrary/plugins/upstream/utils.py will allow you to access the patron's dark mode cookie in templates:

@public
def is_dark_mode_enabled():
    return web.cookies().get('dm', False)

The @public decorator exposes the function to templates. Before this will work, you'll also have to restart the web container.

DarrellRoberts commented 5 months ago

no worries, thank you!

mekarpeles commented 5 months ago

I think

the color pallet pictured is in the realm of reasonable. Use your discretion. When a PR is opened, we can have you or the assignee bring it to a Friday call to get feedback.
you're right that some inverted assets may be useful -- we may be able to do this with css as well, it's worth a test
for now (we can get feedback) a good place for a dark mode toggle may be in the internet archive top bar near the donate & language selector... Something like a (o ) left/right toggle which shows a moon :moon: if not enabled or a:sun:sun to disable. Just an idea for us to try, not a decision

jimchamp commented 4 months ago

9231 creates a proof of concept that toggles the background color of the site whenever the dark mode icon is clicked, and sets a new `dm` cookie. If `dm=True`, then the dark mode class is added to the page's `body` element at render-time. Thanks for putting this together @DarrellRoberts!

As we now need a dark mode color scheme, and visual designs for the toggle button, I'm marking this issue as https://github.com/internetarchive/openlibrary/labels/State%3A%20Blocked. In the meantime, #9231 will be merged into a feature branch named dark-mode. Once this is unblocked, and a developer is ready to implement the designs, they should pull from the dark-mode branch. Of course, someone from staff will have to update dark-mode with the latest code from master first. I'll also be updating the dark-mode branch periodically in order to avoid the branch becoming too out-of-date.

internetarchive / openlibrary

Dark Mode #4448

Dark Mode Implementation

CSS

9231 creates a proof of concept that toggles the background color of the site whenever the dark mode icon is clicked, and sets a new `dm` cookie. If `dm=True`, then the dark mode class is added to the page's `body` element at render-time. Thanks for putting this together @DarrellRoberts!

internetarchive / openlibrary

Dark Mode #4448

Dark Mode Implementation

CSS

9231 creates a proof of concept that toggles the background color of the site whenever the dark mode icon is clicked, and sets a new dm cookie. If dm=True, then the dark mode class is added to the page's body element at render-time. Thanks for putting this together @DarrellRoberts!

9231 creates a proof of concept that toggles the background color of the site whenever the dark mode icon is clicked, and sets a new `dm` cookie. If `dm=True`, then the dark mode class is added to the page's `body` element at render-time. Thanks for putting this together @DarrellRoberts!