-
Notifications
You must be signed in to change notification settings - Fork 2
add dark mode in Demo #184
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
Conversation
38d7e84 to
16dd567
Compare
apps/demo_web/src/app/layout.tsx
Outdated
| const themeInitScript = ` | ||
| (function() { | ||
| try { | ||
| const stored = localStorage.getItem('theme'); | ||
| let preference = 'system'; | ||
| if (stored) { | ||
| const parsed = JSON.parse(stored); | ||
| preference = parsed.state?.preference || 'system'; | ||
| } | ||
| let theme = preference; | ||
| if (preference === 'system') { | ||
| theme = window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'; | ||
| } | ||
| document.documentElement.setAttribute('data-theme', theme); | ||
| } catch (e) {} | ||
| })(); | ||
| `; | ||
|
|
||
| export default function RootLayout({ | ||
| children, | ||
| }: Readonly<{ | ||
| children: React.ReactNode; | ||
| }>) { | ||
| return ( | ||
| <html lang="en"> | ||
| // To prevent flickering in dark mode or light mode in Next.js, an inline script is required. | ||
| // The reason is that even when using LayoutEffect in React, it executes after hydration. | ||
| <html lang="en" suppressHydrationWarning> | ||
| <head> | ||
| <script dangerouslySetInnerHTML={{ __html: themeInitScript }} /> | ||
| </head> | ||
| <body |
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.
Added a line script to prevent flickering before hydtraion.
| import { IntegrationCard } from "./integration_card/integration_card"; | ||
| import { useViewState } from "@oko-wallet-demo-web/state/view"; | ||
| import { ThemeButton } from "@oko-wallet-demo-web/components/theme/theme_button"; | ||
| import { Spacing } from "@oko-wallet-common-ui/spacing/spacing"; |
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.
Invalid import
| | "quaternary-solid"; | ||
| | "quaternary-solid" | ||
| | "brand-solid"; | ||
| titleColor?: BaseTypographyProps["color"]; |
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.
Please try not to use the "Indexed Access Type" of TypeScript.
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.
Is there any particular advantage to using BaseTypographyColor over using BaseTypographyProps["color"]?
| content, | ||
| hideFloatingArrow, | ||
| backgroundColor = "primary-solid", | ||
| titleColor = "white", |
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.
I'm also against the idea of having parameter default value
| export const THEME_STORAGE_KEY = "theme"; | ||
| export const THEME_ATTRIBUTE = "data-theme"; | ||
|
|
||
| export const themeInitScript = ` |
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.
✔️
Pull Request
CONTRIBUTING.mdand followed the guidelines.Summary
The default setting follows the system preference. Clicking the theme button once or more saves the selected theme to local storage.
Links (Issue References, etc, if there's any)