-
Notifications
You must be signed in to change notification settings - Fork 119
feat(docs): fix theme flicker #4271
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
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -49,6 +49,8 @@ const LivePreview: React.FC<React.PropsWithChildren<LivePreviewProps>> = ({ | |||
|
|||
const overflow = showOverflow ? "visible" : "auto"; | |||
|
|||
if (!previewTheme) return null; |
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 am now delaying the Live code preview rendering until we have correctly identified the theme. I could not fix the Theme provider with CSS variables as it's nested an needs to cahgne independent of the actual app. This was the only workaround to not getting the previews to do the flicker. Won't effect SEO
const [componentMounted, setComponentMounted] = React.useState(false); | ||
|
||
const setMode = (mode: ValidThemeName): void => { | ||
SimpleStorage.set("theme", mode); |
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.
No need to use local storage, all done via cookies now
@@ -1,4 +1,6 @@ | |||
import { datadogRum } from "@datadog/browser-rum"; | |||
import "@twilio-paste/design-tokens/dist/themes/twilio-dark/tokens.data-theme.css"; | |||
import "@twilio-paste/design-tokens/dist/themes/twilio/tokens.custom-properties.css"; |
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.
By using the tokens.custom.properties.css
instead of tokens.data-theme.css
it will set this theme to be applied by default and only overridden when the document.body.data.theme = "twilio-dark"
View your CI Pipeline Execution ↗ for commit 84cefa7.
☁️ Nx Cloud last updated this comment at |
<script | ||
type="text/javascript" | ||
// eslint-disable-next-line react/no-danger | ||
dangerouslySetInnerHTML={{ | ||
__html: ` | ||
let parts = typeof document !== "undefined" && document?.cookie.split("paste-docs-theme="); | ||
let cookie = parts.length == 2 ?parts?.pop().split(";").shift() : null; | ||
if(cookie){ | ||
document.body.dataset.theme = cookie; | ||
} | ||
else if(window !== "undefined" && window.matchMedia && window.matchMedia("(prefers-color-scheme: dark)").matches){ | ||
document.body.dataset.theme = "twilio-dark"; | ||
} | ||
`, | ||
}} | ||
/> |
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 tried doing this using the Next Script element but I didn't have any luck, flicker was there just shorted. Doing it this way was the only thing to ge tit to work
Size Change: 0 B Total Size: 1.12 MB ℹ️ View Unchanged
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 84cefa7:
|
Paste
|
Project |
Paste
|
Branch Review |
use-css-themes-next-js
|
Run status |
|
Run duration | 03m 53s |
Commit |
|
Committer | Kristian Antrobus |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
67
|
View all changes introduced in this branch ↗︎ |
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.
beautiful
Paste
|
Project |
Paste
|
Branch Review |
use-css-themes-next-js
|
Run status |
|
Run duration | 06m 58s |
Commit |
|
Committer | krisantrobus |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
125
|
View all changes introduced in this branch ↗︎ |
Updated the site to use CSS variables instead of static themes to fix theme flicker