medusajs / nextjs-starter-medusa

A performant frontend ecommerce starter template with Next.js 14 and Medusa.
https://next.medusajs.com/
MIT License
1.8k stars 507 forks source link

Dark mode incompatibility #253

Closed xyzones closed 9 months ago

xyzones commented 9 months ago

With tailwindcss>3.3.3, the design breaks during active dark mode.

light dark
fuse-thanakorn commented 9 months ago

I got the same problem, any update ?

VariableVic commented 9 months ago

Hey guys @xyzones @fuse-thanakorn,

I think there was a bug in Medusa UI that caused some dark mode issues. Should be solved in later versions. Could you try updating Medusa UI to >2.4.1? "@medusajs/ui": "^2.4.1"

xyzones commented 9 months ago

@VariableVic the dark:... is missing from lot of places. so I don't think was only a @medusajs/ui issue

I have update the package and it is still not working.

VariableVic commented 9 months ago

The Starter doesn't support dark mode, so any dark: classes present are leftovers from an early version.

I can't replicate the issue with the package versions you mentioned. What browser are you using?

fuse-thanakorn commented 9 months ago

@VariableVic For me Safari and Chrome still got that same problem.

xyzones commented 9 months ago

@VariableVic this is what we say that due to some leftover or bugs the design breaks in dark mode. Safari, chrome same issue

Bernix01 commented 9 months ago

For me this issue happened when I updated tailwind above 3.3.3 as the issue description says

ericwaetke commented 9 months ago

I’ve got the same problem even with 2.4.1. And I also don’t think it’s any dark: leftovers, as there are no dark: classes in the code.

However, there is a layout.css file being generated, that sets the color variables based on the css prefers-color-scheme: dark setting.

Bildschirmfoto 2024-02-26 um 15 34 24

It’s because the ui-preset tailwind plugin is looking at the config to see, if the darkMode: 'class' property is set in the tailwind.config.js. If not (as it is by default), it overrides the color variables based on the system color scheme.

So you can fix it by explicitly saying the dark-mode isn’t set based on the system:

module.exports = {
  darkMode: 'class',
  presets: [require("@medusajs/ui-preset")],
  …
}

I could also create a PR to make this the default behaviour for freshly created nextjs-starters, if that’s wanted

VariableVic commented 9 months ago

@ericwaetke great catch, I'd happily accept that PR!

xyzones commented 9 months ago

I don't think you understand the problem. We need to darkMode and it is simply not working.

VariableVic commented 9 months ago

We need to darkMode and it is simply not working.

What do you mean exactly? Are you trying to add dark: classes and they aren't picking up? Please elaborate on what you're trying to achieve and what is not working as expected.

xyzones commented 9 months ago

@VariableVic just look the original post pictures with darkmode the whole store is unusable

VariableVic commented 9 months ago

Like I said, this Starter doesn't support dark mode out of the box. It's light mode only. There's 2 things you can do here:

  1. Implement @ericwaetke's fix to prevent the store from switching to dark mode entirely. This will enforce light mode regardless of the user settings.
  2. Or, if you want to use dark mode: implement it yourself. You can use Medusa UI classes like bg-ui-bg-base - they will switch based on the user system setting. As you can see in your screenshot, some components are already using these, and thus reacting to dark mode.
xyzones commented 9 months ago

@VariableVic so no intention to make this available? I have solved this went I opened, so is not really affecting me, however this would be surely much appreciated by the community ;)

VariableVic commented 9 months ago

It's not a priority as of now, but we might add it later. Feel free to add it as an idea to GH discussions!