feature/theme-toggle fixes #2909 #2996
Conversation
|
minor fixes for firefox |
|
@jgonggrijp I am open to feedback for improvements as well. |
elkcityhazard
changed the title
There was a problem hiding this comment.
Thank you for jumping onto this so eagerly, @elkcityhazard!
I wonder whether a manual toggle is really needed. With the (prefers-color-scheme: dark) media query, doesn't everyone automatically get what they want? That being said, I like the way it looks and the fact that it remembers the setting on reload.
Of all the styling being applied in dark mode, only the filter on the logo image seems to be taking effect, and the wrong effect at that. This is what I see:
Computer set to prefer light, toggle set to light
Computer set to prefer light, toggle set to dark
Computer set to prefer dark, toggle set to dark
Computer set to prefer dark, toggle set to light
These screenshots were taken with Firefox. With Safari, I see the same thing, except that the toggle isn't visible.
I would suggest saving yourself some complexity. Remove the toggle and just rely on the media queries. If you really want to hold on to the toggle, however, I think it still needs some refinements:
- Make sure the toggle is set to the actual mode initially. I opened the page while the computer was set to dark mode. The logo image was all white (i.e., "dark", not as intended but as expected from the above screenshots). So the page was assuming "dark", but the toggle was switched to the left side with the sun icon.
- Keep the toggle in sight when users scroll down, for example with
position: fixed. There are direct on-page links to each function, so a user landing on the page by following such a link will not see the toggle otherwise.
|
@jgonggrijp are you loading just the index.html? I moved the static assets into a static dir and am running locally using python http.server to serve the files. loading index.html with firefox isn't working out for me either. If it's easier I can refactor out the toggle and just use here is what I am seeing for dark mode on firefox at least:
|
|
@jgonggrijp removed unnecessary Javascript, removed unnecessary static dir, moved stylesheet into head of html. awaiting feedback. |
There was a problem hiding this comment.
Much better!
I didn't think of running a http server. It is good that you removed this requirement. @jashkenas designed this project (and related ones from the same era such as Backbone) with a low-tech attitude: people should need as few steps as possible to view the documentation on their own computer, run the tests, try the bleeding-edge version etcetera. I would not make that choice if I were to invent the project myself, but I respect this approach and would like to keep it all working.
Three requests:
- Please preserve the coloring of the logo. I think this should work with the CSS filter that I suggested in my previous review.
- (optional) As far as I'm concerned, the dark mode background could be darker. But if this offends your sense of beauty, I am willing to let go of this request.
- Align the indentation with the rest of the code.
|
Released in 1.13.7. |
a crack at adding theme toggle for underscores documentation website