privacyguides / privacyguides.org

Protect your data against global mass surveillance programs.
https://www.privacyguides.org
Creative Commons Attribution Share Alike 4.0 International
2.76k stars 207 forks source link

Dark mode flashes white #1930

Closed zetix closed 1 year ago

zetix commented 1 year ago

Really jarring when trying to browse in dark mode with the random flash bangs.

2023-01-08_20-44-46_firefox

dngray commented 1 year ago

Hmm, this definitely seems to be a bug, will have to look into what is causing that.

noClaps commented 1 year ago

It happens because the data-md-color-scheme property of the <body> tag has the value of default when the page first loads, which means that it will display the default color scheme before updating to the slate theme after loading.

I'm facing the same problem on my blog site, will update if I find a solution.

Edit: One solution I've found is to default to dark theme, then update to light theme if prefers-color-scheme: dark is false. That should work if scheme in mkdocs.production.yml:

theme:
  palette:
    - media: "(prefers-color-scheme)"
      scheme: default
      accent: deep purple

is changed to slate from default. The only effect of this that I can see is that anyone browsing the site without JS enabled will see it in the dark theme instead of the light theme. I unfortunately can't see if that actually works, but maybe a team member could try.

jonaharagon commented 1 year ago

Screenshot 2023-02-09 at 20 02 35

@noClaps unfortunately that seems to have the opposite problem in light mode, which I'm not a big fan of.

noClaps commented 1 year ago

That's a good point. However, it's probably a bit better to show a dark flash when loading a site in light mode than to flashbang someone loading in dark mode.

We can keep the issue open and try to work out a better solution (I need something for my blog anyway since it suffers from the same issue), but in the meantime switch the themes.

I have a few ideas I'll test with my blog tomorrow, and I'll let you know how it goes and if you can implement something similar with MkDocs (I use Astro, but I'm sure it can be ported over somehow).

jonaharagon commented 1 year ago

I'm a little confused by this bug because I'm not seeing this behavior on https://squidfunk.github.io/mkdocs-material/.

noClaps commented 1 year ago

Someone in the Matrix room also suggested using sessionStorage to store the theme state so that it doesn't have to reload it every time you switch pages. That should get rid of the theme reloading and flashing every time you switch between pages after the site has loaded for the first time.

My implementation: https://github.com/noClaps/blog/commit/29e1f1d8cdee69e9e31b2210a73a4a9b19d87bbd

noClaps commented 1 year ago

I'm a little confused by this bug because I'm not seeing this behavior on https://squidfunk.github.io/mkdocs-material/.

Well it's loading the HTML before hydrating it with JS. Since the HTML is in light mode by default, the page renders in light mode and then when its hydrated, the JS triggers and switches it to dark.

The solution I found was to switch the default HTML to dark mode, but like you mentioned, that causes the opposite problem in light mode.

As for the mkdocs-material site, it just seems to hydrate more quickly. Not sure how or why, I'll have to look into that tomorrow.

noClaps commented 1 year ago

@jonaharagon

Found a fix https://github.com/noClaps/blog/commit/85b336519deff73ba5b351636f9755103411ab79 https://github.com/noClaps/blog/commit/acba7b8bb2dddbf3158df7529e2d63a642749375

In the case of the PG website, the media query is used to update the data-md-color-scheme property, which is then used to update the theme, as I said before. It has to go through a lot of steps to be able to display the correct theme to the user. If there's a lot of things to load, like there are on the PG website, those steps can take time, hence the flash.

The way I did it was to have media queries set the default theme, and then override that with the theme toggle. That way, the default theme can be loaded even before the site is hydrated. However, in the case of PG, this seems to depend more on the upstream implementation of the theme toggle, since that entire functionality is written there. You might be able to implement this fix by writing out all the CSS variables into their respective media queries.

blacklight447 commented 1 year ago

@noClaps would you be willing to make a PR for this? :)

noClaps commented 1 year ago

I can try

noClaps commented 1 year ago

From what I can tell, the main issue seems to lie in the

- media: "(prefers-color-scheme)"
  scheme: default
  accent: deep purple
  toggle:
    icon: material/brightness-auto
    name: Switch to light mode

bit, since it seems to work just fine in my local version with the manually set themes. I can't access and test the prefers-color-scheme setting without access to the Insiders version of Material for MkDocs.

kamilkrzyskow commented 1 year ago

I don't get those "flashes" - late CSS rendering on the MkDocs Material site (which uses the 3 step Insiders toggle) and neither on my teams site (which uses the 2 step Public toggle), However, I confirm getting them on your site (in white) and slightly less (in dark) on the FastAPI docs page.

I did some benchmarking with the Chrome Dev Tools removing practically everything from your site and there was no change, therefore I don't think that the amount of content is causing that. I could try to make further blockades and remove the calls for the images altogether (150+ requests), but I don't feel like it's the correct way. What I noticed too is that my team and the Material site use Github Pages as the hosting, and FastAPI and PrivacyGuides use Netlify. So maybe you could try hosting it on GitHub for a second, but I guess it's not that either because it seemed to flash on localhost too πŸ€”. I doubt lazy loading of the images would help but it's worth a try as the majority of requests are in fact <img> with SVGs, maybe injecting them would help πŸ€”.

https://user-images.githubusercontent.com/34622465/221956201-5465af3b-4bca-40e8-83ab-612b1d72b613.mp4

https://user-images.githubusercontent.com/34622465/221956193-2ea1a34a-71c3-4587-bbf0-26a2c0873346.mp4

noClaps commented 1 year ago

I didn't think to check the network tab, thanks for doing that analysis.

I did some benchmarking with the Chrome Dev Tools removing practically everything from your site and there was no change, therefore I don't think that the amount of content is causing that.

I thought that loading everything in was slowing down hydration, but if blocking it all still causes the same issue, then it's probably something else.

What I noticed too is that my team and the Material site use Github Pages as the hosting, and FastAPI and PrivacyGuides use Netlify. So maybe you could try hosting it on GitHub for a second, but I guess it's not that either because it seemed to flash on localhost too

I'm not sure the hosting provider matters too much in this case, since like you said, it has the same effect on localhost (which should theoretically load faster than any hosting provider anyway).

kamilkrzyskow commented 1 year ago

I did some more testing. This time on localhost, and just about when I wanted to record the video, the i18n redesign got deployed πŸ˜… and the repository removed the non-insiders support, you work FAST πŸ˜† anyways... tested locally with the bfaba1cd11b71b2ff8c7c05aa9ecfe95e3bb73fd commit.

There is no flash on localhost either, and I even injected Insiders files into the Public version just to be sure it's not a weird quirk with the Insiders only 3 step palette selection and this site.
I truly believe it's some sort of quirk with Netlifly that GitHub pages doesn't have. Maybe it's their HTTP compression, but not sure on that point, but it's more compressed opposed to what GitHub serves. Apart of that I'm out of ideas, I'm not sure if there is anything else to test here

I've created a temporary fork with a deployment of the previously mentioned commit and injected files, I have no flashes there either, I'll remove it tomorrow. https://kamilkrzyskow.github.io/TempFork/

https://user-images.githubusercontent.com/34622465/222047397-c072058c-a1fe-44d7-8cdb-0c5ba3fdc5a8.mp4

kamilkrzyskow commented 1 year ago

Then again in https://github.com/privacyguides/privacyguides.org/issues/1930#issuecomment-1425078289 localhost also flashed in reverse for @jonaharagon, maybe the issue lies in the browser or something else entirely. Or even maybe it is something related to Insiders features that are being used on build and then injected into HTML πŸ€” and my simple file injection of the external files wasn't enough to trigger it. But currently it's impossible for me to test that because I don't have Insiders.

jonaharagon commented 1 year ago

Unfortunately I really can't confirm what you're finding here. With the current build on localhost:

https://user-images.githubusercontent.com/3637842/222159046-2f088b7c-810d-4542-9ad4-d19af01b4b24.mp4

Going to look at your fork next...

jonaharagon commented 1 year ago

It occurs to me that we do still build to GitHub Pages: https://privacyguides.github.io/privacyguides.org/ (where this issue persists). I definitely don't see it happening on your fork though.

Can you share/commit the changes you made on your end?

kamilkrzyskow commented 1 year ago

I'm not at my PC currently, but there were no flashes even without changes in that commit. All I did before the deployment was to replace the palette selector in the mkdocs.yml from the 2 to 3 step variant, and replace the bundle.js, main.css, and palette.css to the one Insiders uses.

The flashing on your GitHub deployment disproved my Netlifly accusations. However it kinda proved that there is something in your Insiders build that wasn't in the Public build.

I would recommend to do testing on localhost, rebuild the page removing features/plugins 1by1 until you reach the same mkdocs.yml config that worked with the Public mkdocs-material.

I will later do a diff compare of the HTML source from your GitHub deployment and mine.

jonaharagon commented 1 year ago

I've reduced mkdocs.en.yml to the following config and still see this issue. Wonder if it's an override we have in /theme then, going to check those out next.

docs_dir: '../docs'
site_url: "https://www.privacyguides.org/en/"
site_dir: '../site/en'

site_name: Privacy Guides

theme:
  name: material
  custom_dir: ../theme
  language: en
  palette:
    - scheme: default
      accent: deep purple
      toggle:
        icon: material/brightness-5
        name: "Switch to dark mode"
    - scheme: slate
      accent: amber
      toggle:
        icon: material/brightness-2
        name: "Switch to system theme"

nav: [...]
kamilkrzyskow commented 1 year ago

Awesome 😸 So my second or third hunch was correct. Nonetheless, I'm quite surprised that the issue was in the template HTML overrides, because it should be included even without the Insiders build. I was holding strong to the Netlifly idea, because FastAPI had similar, but slightly less severe issues on my end, but it doesn't replace any template partials.

You might be interested in my "How to dynamically override templates with hooks?" guide: https://github.com/squidfunk/mkdocs-material/discussions/5060

It would allow to detect breaking changes in the template overrides faster. I understand that the issue here was a deprecated HTML template (? I didn't do a diff change, since you already fixed it, so only assuming). The hook script was updated after the guide was made, to address an issue with the backup methodology, it's still not in the final form, so I didn't update the guide yet, however it was and still is fully functional.

jonaharagon commented 1 year ago

Yeah, I still have no idea whatsoever how your build worked without the changes there though.

I'll look at your guide though, the problem you're solving there is exactly the same thing I was just trying to do a few days ago (use flags for the language picker, because someone noted they didn't recognize what the translate icon was), so at the very least that's really convenient for us πŸ˜„

kamilkrzyskow commented 1 year ago

Great to hear 😺 In the meantime you could use the :material-earth: in your config.theme.icon.alternate it's quite good πŸ‘ The issue which I haven't fixed yet is the <img> tag injection instead of inline SVGs, this leads to a delay where there is no icon being shown. I wrote, in the guide:

The current version of the hook doesn't support inline SVG's, and there is no fallback for the case when the remote images won't load, so the selector button will be empty when that happens. In that regard the static override solution might be better as it hosts the images in the docs tooπŸ˜„.

And only much later I connected the dots in my head, that the hook allows to replace the

{% include ".icons/" ~ icon ~ ".svg" %}

with

{% include ".icons/flags/language_code.svg" %}

and as this is a Python script it can totally just download/scrape some ZIP icon-pack or flag SVGs and place them in both the .cache directory (to avoid re-downloading) and the .icons directory respectively. However, I don't have any plans to do such thing currently.

When I was saying the current hook isn't in the final form, I meant that after introducing changes to the backup methodology and introducing namespaces to share information between hooks, I created a bad case of redundancy, because every hook needs their own BackupManager class, which is of course not ideal and it would be better to separate the Management into a separate overrides_manager.py hook that would be then connected to in other hooks, aka create hooks to a hook πŸ˜† Anyways, the idea's on the back burner.

privacyguides-bot commented 1 year ago

This issue has been mentioned on Privacy Guides. There might be relevant details there:

https://discuss.privacyguides.net/t/v3-3/11950/1