abereghici / remix-themes

An abstraction for themes in your Remix app.
https://www.npmjs.com/package/remix-themes
MIT License
123 stars 14 forks source link

data-theme attribute sets the wrong theme on it's first use. #12

Closed dev-xo closed 2 years ago

dev-xo commented 2 years ago

Describe the bug

data-theme attribute, sets the wrong theme on it's first use. Theme is being set correctly, on it's second use. Current behavior: First use: sets 'light' theme (current theme). => Second use: sets 'dark' theme (expected theme).

Also on first page load, I get this common error: Warning: Prop data-theme did not match. Server: "null" Client: "dark" html.

Additional Info: PreventFlashOnWrongTheme is correctly set on 'head' tags.

Your Example Website or App

http://localhost:3000/

Steps to Reproduce the Bug or Issue

  1. Clear cookies from the website: localhost (in this case).
  2. Set a data-theme attribute on the top html element (root.tsx).
  3. Import useTheme method, and use it for a newly clickable button.
  4. Navigate to the website, and click the newly created button.
  5. Check dev console (Inspector Mode) for the top html element and it's data-attribute values.

Expected behavior

data-theme attribute should set the opposite preferred theme on it's first use. Example: prefers-color-scheme: light => theme should change to 'dark' on it's first use.

Screenshots or Videos

No response

Platform

Additional context

You guys are doing an amazing job with the module. Hope it keeps getting better and better, because for sure it will be used by a lot of people soon or later.

Thanks!

abereghici commented 2 years ago

Hey @deveIoper-io, thank you for opening this issue. I created a PR https://github.com/abereghici/remix-themes/pull/13 to fix this issue. Any feedback will be appreciated!

dev-xo commented 2 years ago

@abereghici Didn't checked the code yet, but I used to implement my own dark theme on Remix, and instead of using document.addClass (Or something like this) I used to do: document.hasAttribute | document.setAttribute('data-theme', theme)

That was setting the data-theme attribute from the server correctly. Maybe we can check if the data-theme is set by default, replacing className for data-theme as a default behavior.

I'll check It later, maybe I can help with it. As I sayd yesterday, thanks for your work. This module will be much used soon or later!

Remix is just amazing!