jupyter-book / myst-theme

Packages for creating MyST websites themes using React and Remix
https://jupyter-book.github.io/myst-theme/?path=/docs/components-introduction--docs
MIT License
16 stars 14 forks source link

Light / dark mode does not persist between pages in static export #319

Closed choldgraf closed 2 months ago

choldgraf commented 8 months ago

Description

The light and dark mode changes back to the default when you navigate between pages. This is on an HTML build and can be demoed here:

https://2i2c.org/report-czi-2021/

For example:

CleanShot 2024-03-01 at 18 00 46

Proposed solution

Persist the status of light/dark by using an HTML browser window key variable.

Additional notes

agoose77 commented 8 months ago

@stevejpurves could you move this to myst-theme please?

Architecturally, it looks like myst-theme populates the "default" theme for a server route using a cookie, and local manipulation of the theme invokes an API request to update the cookie.

I think we can move this to localStorage for static builds -- Remix will have SSR'd the page, so we can safely read from localStorage. The question will be how deep this goes; can we abstract out the API request in favour of something that reads/writes localStorage. Perhaps a different provider should be used.

choldgraf commented 8 months ago

If it's helpful, here's how the pydata theme (what jupyter book builds off of) handles this:

https://github.com/pydata/pydata-sphinx-theme/blob/b9719c2a789eca3a193d4fd64ab71d6283a923db/src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js#L7-L33

welcome[bot] commented 8 months ago

Thanks for opening your first issue here! Engagement like this is essential for open source projects! :hugs:
If you haven't done so already, check out EBP's Code of Conduct. Also, please try to follow the issue template as it helps other community members to contribute more effectively.
If your issue is a feature request, others may react to it, to raise its prominence (see Feature Voting).
Welcome to the EBP community! :tada:

stevejpurves commented 8 months ago

localstorage is not a good solution for SSR but for static builds we can probably opt out of the cookie and do local storage instead

rowanc1 commented 8 months ago

As @stevejpurves mentioned, we should continue using cookies for SSR, but switch to local storage for the static render. As local storage is not present on the server, there is a flicker which can be avoided when the theme preference is treated as shared state between server/client (best practice in that scenario).