fix: Reduce tearing issues #1113
Conversation
| const { path } = useLocation(); | ||
|
|
||
| if (!isDocPage(path) || version === LATEST_MAJOR) { | ||
| if (version === LATEST_MAJOR) { |
There was a problem hiding this comment.
Was a bit of a problem as useLocation, upon navigating from (say) / -> /guide/... immediately updates with the new path, but as the route wasn't loaded yet, name & version would be undefined. This would result in the banner flashing upon nav.
We could correct that here, but it's better to conditionally render the entire component. OldDocsWarning should never be loaded on anything but guide pages.
| useTitle(meta.title); | ||
| useDescription(meta.description); | ||
|
|
||
| const hasSidebar = meta.toc !== false && isDocPage(path); |
There was a problem hiding this comment.
Redundant; the home page is the only one with meta.toc == false, and the sidebar only appears on /guide/* pages. Better to switch it to isGuide as that's essentially what we're checking.
|
|
||
| export function FakeSuspense(props) { | ||
| this.__c = childDidSuspend; | ||
| return this.state && this.state.suspended ? props.fallback : props.children; | ||
| } | ||
|
|
||
| function childDidSuspend(err) { | ||
| err.then(() => this.forceUpdate()); | ||
| } |
There was a problem hiding this comment.
Not necessarily a tearing issue, but something that needed to be fixed. this.state.suspended isn't ever truthy, and so the fallback doesn't get rendered. Most obviously seen on the home page, that first code block is gets emptied (and so collapses down) then repopulates once prism runs locally. Pretty jarring.
Tbh, I rather mindlessly copied this from #793 and didn't look too closely at it. Swapping it out for standard Suspense from preact/compat seems reasonable? Dunno if there's something I'm missing though.
JoviDeCroock
left a comment
JoviDeCroock
left a comment
There was a problem hiding this comment.
Is there an opportunity to equalise this more in iso?
|
Not quite sure; my gut says this is more of an API issue. I really don't love that there's two separate hooks/contexts without clear boundaries on when to use which (besides the few bits specific to one or the other). I think this is always going to cause problems as there's always going to be some separation. Maybe there's a way to land an improvement without big changes though, will take a better look soon. |
useLocationfrompreact-isofeels a bit problematic, as it fires significantly earlier thanuseRoute. Don't know if it's quite to the point of being a footgun, but it causes some problems here.