-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
feat(#750): add themestore and usetheme #881
Conversation
Hi 👋 Here's a preview environment 🚀 https://next-reworkd-agentgpt-881.env.ergomake.link Environment Summary 📑
Questions? Comments? Suggestions? Join Discord. Click here to disable Ergomake. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This reverts commit f7835b8.
next/src/hooks/useTheme.ts
Outdated
return theme === PREFERRED_THEME || (theme === "system" && preferredThemeMatches); | ||
} | ||
|
||
export function handleTheme(theme, matchObj?) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use arrow syntax. Also, should we have type params for these functions
next/src/hooks/useTheme.ts
Outdated
@@ -0,0 +1,41 @@ | |||
import { type Theme, THEMES } from "../types"; | |||
import { useEffect } from "react"; | |||
const PREFERRED_THEME = "dark"; // preferred theme must be dark for Tailwind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come?
next/src/hooks/useTheme.ts
Outdated
useEffect(() => { | ||
const preferredTheme = window.matchMedia(`(prefers-color-scheme: ${PREFERRED_THEME})`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here? What if their preferred theme isn't dark?
next/src/hooks/useTheme.ts
Outdated
const classList = document.documentElement.classList; | ||
|
||
if (isPreferredTheme(theme, matchObj)) { | ||
classList.add(PREFERRED_THEME); | ||
} else { | ||
classList.remove(PREFERRED_THEME); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats going on here, worth some comments?
onRehydrateStorage: () => { | ||
return (state, error) => { | ||
if (error) { | ||
console.error("an error happened during hydration. ", error); | ||
} else { | ||
handleTheme(state ? state.theme : "system"); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, so this way even if we have local storage we won't run into rehydration issues with SSR? If so, could also use these in the other stores (Had to use a separate hook for them to ensure there were no issues)
No description provided.