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

Gatsby SSR colorMode Fix #1685

Closed
wants to merge 4 commits into from

Conversation

fcisio
Copy link
Collaborator

@fcisio fcisio commented Apr 26, 2021

This PR follows the issue: #1602

Also extends on my previous PR #1639

I wasn't able to repro the issue inside the package, but basically replicated what I do on the user-side.
It should work, and once there is a canary version, I'll be able to test in my own projects.

Let me know what you guys think 👌


I additionally fixed some things that were breaking Gatsby builds internally

@vercel
Copy link

vercel bot commented Apr 26, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/systemui/theme-ui/CmvUQwnPL7aAeYbnhWijvNWfMd2r
✅ Preview: https://theme-ui-git-fork-fcisio-gatsby-ssr-colormode-fix-systemui.vercel.app

@fcisio
Copy link
Collaborator Author

fcisio commented Apr 28, 2021

Alright 😅
Diving deeper into this, I realized I don't really have a fix.

I did draft one, but it only works when local storage is turned off.

I'm not sure what causes the issue in the first place, but my guess is it's from preferredMode and modeFromClass.

Steps to reproduce:

  1. build docs
  2. serve docs
  3. delete the theme-ui-color-mode key from the storage
  4. navigate to http://localhost:9000/home/

A pink div should show up on dark mode (if it's the preferredMode). The issue is only present on the first load. If you switch to dark mode, then all works fine.

If I remember right, I first saw this issue surface around the time useColorSchemeMediaQuery default was changed to true. We could probably trace back what changes are causing this.

If anyone, wants to have a go at this, I'm kinda stuck for now.

@hasparus
Copy link
Member

Closing, because this PR is a leftover. We fixed this already on develop (and fcisio reviewed it)

@hasparus hasparus closed this Aug 11, 2021
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.

2 participants