Skip to content

Conversation

longnguyen2004
Copy link
Contributor

Description

Making currentTheme reactive, so the wrapper div class syncs up with the html class. Partially fixes #334

Before submitting the PR, please make sure you do the following

  • Read the Contributing guide
  • Prefix your PR title with [@svelteui/core], [@svelteui/actions], [@svelteui/motion], [@svelteui/core], [core], or [docs].
  • Include a description of the changes made in the PR description and in the commit messages.
  • Include screenshots/videos of the changes made.
  • Verify that the linter and tests are passing, with yarn lint and yarn test or just run yarn prepush.

@BeeMargarida
Copy link
Member

Hi! Thank you for this contribution! I'll not immediately merge since I want to analyze the issue again, since I'm not 100% sure what the cause yet.

@BeeMargarida BeeMargarida added bug Something isn't working styleAPI Involving changes to the StyleAPI labels Apr 29, 2023
@BeeMargarida
Copy link
Member

Sorry, been busy with work, will check this out this week!

@BeeMargarida
Copy link
Member

I can't seem to replicate the issue that this PR seems to fix. Can you?

@longnguyen2004
Copy link
Contributor Author

Here's a StackBlitz repro of the issue https://stackblitz.com/edit/svelteui-dark-mode-bug-repro
And here's a video describing the bug

2023-05-13.17-47-07.mp4

Basically, if the initial theme state is dark, then some components will be stuck in the dark state, because currentTheme is not reactive, so the dark-theme class is stuck on in the wrapper div.

There's also the issue of the background being initially white, before switching to dark, which is due to this not being SSR friendly. I didn't include a fix for that here, but I've outlined potential fixes for it in #334 (comment)

@BeeMargarida
Copy link
Member

After merging this I'll make a new release, and fixing this for good will be the focus of the next one, I think

@longnguyen2004
Copy link
Contributor Author

While you're at it, have a look at #373 and #370 too, those are trivial fixes

@BeeMargarida
Copy link
Member

While you're at it, have a look at #373 and #370 too, those are trivial fixes

Yap, looking at them now. I'll see if I can fix them and make a release this afternoon

@BeeMargarida BeeMargarida merged commit 4b04313 into svelteuidev:main May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working styleAPI Involving changes to the StyleAPI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initial dark mode state - incorrect dark mode switching in AppShell example
2 participants