fix(live-codeblock): render static codeblock server-side#5754
fix(live-codeblock): render static codeblock server-side#5754slorber merged 15 commits intofacebook:mainfrom
Conversation
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
|
✔️ [V2] 🔨 Explore the source changes: 3466069 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61717dd32158f9000846785a 😎 Browse the preview: https://deploy-preview-5754--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5754--docusaurus-2.netlify.app/ |
|
This is a good improvement. I think in addition to this it may be desirable to put the wrapper that says 'live editor' and such so the layout doesn't jitter too much. If it could somehow be rendered instead of the prism editor. |
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Good point. That makes this fix slightly more complex, but looks good |
|
IMHO we should render the whole live layout on the server, with the playground showing a temp spinner (we don't have a spinner comp yet, but |
| </LiveProvider> | ||
| </div> | ||
| // https://github.com/facebook/docusaurus/issues/5747 | ||
| <BrowserOnly |
There was a problem hiding this comment.
I'm not sure exactly why we need this fallback.
Without BrowserOnly, what exactly doesn't work? What are the components throwing, not working or leading to weird side-effects?
There was a problem hiding this comment.
#2986 🧐
We even have a dogfood test here: https://deploy-preview-5754--docusaurus-2.netlify.app/tests/pages/markdownPageTests#test
Okay, let's make another jk, I will just make rendering the Live comps browser only. Edit. The code block doesn't work well with the additional output. I will figure out how to make the style more in line with the editor comp. |
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
| <div className={styles.playgroundPreview}> | ||
| <LivePreview /> | ||
| <LiveError /> | ||
| <BrowserOnly fallback={<div>Loading...</div>}> |
There was a problem hiding this comment.
Should we translate this? Feel like for a normal user this won't even be visible.
There was a problem hiding this comment.
Doesn't seem very important. Eventually, we'll improve it later and add some kind of CSS spinner.
| </Translate> | ||
| </Header> | ||
| <LiveEditor className={styles.playgroundEditor} /> | ||
| <BrowserOnly |
There was a problem hiding this comment.
Going to restore this, because LiveEditor can render on the server, and it is better to not use an alternate rendering (your solution also worked but produced more layout shifts due to different padding)
| Before | After |
|---|---|
|
|
|
There was a problem hiding this comment.
Not sure if that would lead to the context's value being updated, but maybe because the consumer isn't rendered yet, it doesn't actually cause the re-render problem?
There was a problem hiding this comment.
The double-rendering issue that was reported (#2986) was about the live preview, not the editor.
There was a problem hiding this comment.
Yes, I thought rendering the editor would also trigger a re-render once the consumer (output) gets mounted. But I think I was just getting it wrong🌚
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
|
@Josh-Cena @ntucker Tell me if you see any issue on one of these 2 pages:
For me, it looks good and reduces layout shifts with/without JS |
|
The live editor is not remounted in dark mode, so it stays white. And if we remount it, we will trigger a re-render in the consumer as well 😅 |
|
Good catch 🤪 Remounting just the editor does not trigger double-render in the live preview and seems to fix the theming issue Let me know if you see another issue now |
|
LGTM now. Thanks for the polishment :D |
4 participants
Motivation
Resolve #5747. I think a static code block pretty closely resembles the live code block, so we can just use that.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
With JS disabled:
This is not a fix to make live editor any more functional without JS though, so I don't think making it editable or using the actual
LiveEditorcomp is worthwhile. As long as we get the code block server-side rendered, it should be good.