fix: preload documents on hover #8110
Merged
+34
−13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
No changes to documentation |
stipsan
force-pushed
the
preload-on-hover
branch
from
8a8513a to
01e6688
Compare
|
Component Testing Report Updated Dec 20, 2024 1:59 PM (UTC) ❌ Failed Tests (1) -- expand for details
|
⚡️ Editor Performance ReportUpdated Fri, 20 Dec 2024 14:01:10 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
stipsan
force-pushed
the
preload-on-hover
branch
2 times, most recently
from
8e35af9 to
2a8c6a8
Compare
bjoerge
reviewed
Dec 19, 2024
stipsan
commented
Dec 19, 2024
| @@ -124,14 +127,6 @@ export function PaneItem(props: PaneItemProps) { | |||
| documentPresence, | |||
| ]) | |||
|
|
|||
| const Link = useMemo( | |||
There was a problem hiding this comment.
Defining components during render should be avoided whenever possible 🙌
stipsan
force-pushed
the
preload-on-hover
branch
from
2a8c6a8 to
27f93ad
Compare
stipsan
force-pushed
the
preload-on-hover
branch
from
27f93ad to
66c9ddf
Compare
stipsan
commented
Dec 19, 2024
| @@ -89,7 +89,7 @@ export const Pane = forwardRef(function Pane( | |||
| ref.current = refValue | |||
| }, []) | |||
|
|
|||
| useEffect(() => { | |||
| useLayoutEffect(() => { | |||
There was a problem hiding this comment.
This fixes a long standing quirk where clicking on a document sees a weird initial layout shift before the panel is mounted 😮💨
bjoerge
force-pushed
the
preload-on-hover
branch
from
f424f6c to
58c157c
Compare
|
(did a rebase of this branch, fyi @stipsan) |
bjoerge
force-pushed
the
preload-on-hover
branch
from
58c157c to
7d9c5e6
Compare
bjoerge
force-pushed
the
preload-on-hover
branch
from
7d9c5e6 to
e30f1a6
Compare
jordanl17
approved these changes
Dec 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
By preloading the
editStatefor a document it makes for a much snappier experience when navigating documents, here's a video demonstrating both side-by-side (with the new behavior on the left, and no preloading on the right):Screen.Recording.2024-12-19.at.14.33.33.mov
What to review
Is there enough inline context or should I add code comments?
Testing
Existing tests should capture regressions, otherwise the manual testing steps are demonstrated in the video 🙌
Notes for release
Added preloading of documents when hovering them in document list panes. This speeds up the experience of navigating between documents pretty significantly.