-
Notifications
You must be signed in to change notification settings - Fork 119
Fix dark mode flicker docs #4250
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
Changes from 7 commits
67b66e1
b27d1ee
27b1546
95a21bf
49d320b
61772bc
936d03c
ff6a92d
0d609b2
34d32e4
3526bf8
b55612c
35e52a4
7af23e2
9260386
927dece
4cb9ff7
1760cd0
19e0232
26c18ec
2ea9427
196faf0
0d5fe90
801a9ed
3aaa322
4eb27ee
752dd2d
8079b9f
73e7cd1
07a5a8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,9 +11,14 @@ import { SiteSearch } from "../../site-search"; | |
const SiteHeaderSearch: React.FC = () => { | ||
const [isOpen, setIsOpen] = React.useState(false); | ||
const { breakpointIndex } = useWindowSize(); | ||
const isMacOS = navigator.platform.toUpperCase().includes("MAC"); | ||
// navigator is not available in SSR, settign a default until it renders to client | ||
const [isMacOS, setIsMacOS] = React.useState<boolean>(); | ||
const platformTriggerKey = isMacOS ? "Meta" : "Control"; | ||
|
||
React.useEffect(() => { | ||
setIsMacOS(typeof window !== "undefined" && navigator && navigator?.platform.toUpperCase().includes("MAC")); | ||
}, []); | ||
|
||
const onOpen = (): void => { | ||
setIsOpen(true); | ||
}; | ||
|
@@ -68,7 +73,7 @@ const SiteHeaderSearch: React.FC = () => { | |
Search | ||
</Text> | ||
</Box> | ||
{breakpointIndex === 0 ? null : ( | ||
{breakpointIndex === 0 || isMacOS === undefined ? null : ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not render the keyboard shortcuts in the search bar until we know what OS they are using |
||
<> | ||
<Box as="span" color="colorText" aria-hidden="true" marginLeft="space30" lineHeight="lineHeight20"> | ||
<KeyboardKeyGroup {...keyCombinationState}> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,47 @@ | ||
import type { ValueOf } from "@twilio-paste/types"; | ||
import * as React from "react"; | ||
|
||
import { SimpleStorage } from "../utils/SimpleStorage"; | ||
|
||
export type UseDarkModeReturn = [ValidThemeName, () => void, boolean]; | ||
|
||
export const themeCookieKey = "paste-docs-theme"; | ||
|
||
const setCookie = (name: string, value: any, days: number): void => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot use the NextJS cookies tool are this does not run client side and I couldn't get it to work except this way or we add a new cookie dependency |
||
let expires = ""; | ||
if (days) { | ||
const date = new Date(); | ||
date.setTime(date.getTime() + days * 24 * 60 * 60 * 1000); | ||
expires = `; expires=${date.toUTCString()}`; | ||
} | ||
// eslint-disable-next-line unicorn/no-document-cookie | ||
document.cookie = `${name}=${value || ""}${expires}; path=/`; | ||
}; | ||
|
||
const getCookie = (name: string) => { | ||
Check failure on line 19 in packages/paste-website/src/hooks/useDarkMode.tsx
|
||
// eslint-disable-next-line unicorn/no-document-cookie | ||
const value = `; ${document.cookie}`; | ||
const parts = value.split(`; ${name}=`); | ||
if (parts.length === 2) { | ||
const part = parts.pop(); | ||
if (part) { | ||
return part.split(";").shift(); | ||
} | ||
} | ||
}; | ||
|
||
const ValidThemes = { | ||
DEFAULT: "twilio", | ||
DARK: "twilio-dark", | ||
} as const; | ||
|
||
type ValidThemeName = ValueOf<typeof ValidThemes>; | ||
|
||
const isValidTheme = (themeName: ValidThemeName): boolean => { | ||
return Object.values(ValidThemes).includes(themeName); | ||
}; | ||
|
||
export const useDarkMode = (): UseDarkModeReturn => { | ||
const [theme, setTheme] = React.useState<ValidThemeName>(ValidThemes.DEFAULT); | ||
export const useDarkMode = (defaultValue: ValidThemeName = ValidThemes.DEFAULT): UseDarkModeReturn => { | ||
const [theme, setTheme] = React.useState<ValidThemeName>(defaultValue); | ||
const [componentMounted, setComponentMounted] = React.useState(false); | ||
|
||
const setMode = (mode: ValidThemeName): void => { | ||
SimpleStorage.set("theme", mode); | ||
setTheme(mode); | ||
setCookie(themeCookieKey, mode, 365); | ||
}; | ||
|
||
const toggleTheme = (): void => { | ||
|
@@ -34,14 +53,10 @@ | |
}; | ||
|
||
React.useEffect(() => { | ||
const localTheme = SimpleStorage.get("theme") as ValidThemeName; | ||
const cookieTheme = getCookie(themeCookieKey) as ValidThemeName; | ||
|
||
if (window.matchMedia?.("(prefers-color-scheme: dark)").matches && !localTheme) { | ||
if (window.matchMedia?.("(prefers-color-scheme: dark)").matches && !cookieTheme) { | ||
setMode(ValidThemes.DARK); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are setting the default value from the cookie we don't need local storage anymore. We also don't want to use teh document cookie in this hook as we will still get the flickering issue. The reason this works now is that itis inside a useEffect hook and in functions so we know it would already be rendered in the client. |
||
} else if (localTheme && isValidTheme(localTheme)) { | ||
setTheme(localTheme); | ||
} else { | ||
setMode(ValidThemes.DEFAULT); | ||
} | ||
|
||
setComponentMounted(true); | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
An error came up after removing the delayed rendering as navigator is only available running client side.