-
Notifications
You must be signed in to change notification settings - Fork 471
⚡️(frontend) improve perf on upload and table of contents #1662
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
8cb92fa to
e608769
Compare
9cea74d to
5598788
Compare
|
Size Change: +130 B (0%) Total Size: 4.11 MB
|
Ovgodd
left a comment
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.
Nice ! just made a very small proposal ( non blocking )
| /* During collaboration, another user might have updated the block */ | ||
| } | ||
| // Add random delay to reduce collision probability during collaboration | ||
| const randomDelay = Math.random() * 800; |
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.
Smart I like it
src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx
Outdated
Show resolved
Hide resolved
| /** | ||
| * Replace the resource block by a uploadLoader block to show analyzing status | ||
| */ | ||
| const replaceBlockWithUploadLoader = useCallback( |
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.
Would it make sense to extract this into a utility function if we plan to reuse it later?
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.
It is hypothetical, let's do that when the situation would happen.
- the Table of Contents stickiness now covers the full height of the viewport, before it was limited to 100vh - we listen the scroll to highlight the heading in the Table of Contents only when the Table of Contents is open - We debounce the editor change to avoid excessive updates to the Table of Contents
We notices that `context.getChanges` was very greedy, on a large document with multiple users collaborating, it caused performance issues. We change the way that we track a upload by listening onUploadEnd event instead of tracking all changes in the document. When a doc opens, we check if there are any ongoing uploads and resume them. We fix as well a race condition that could happen when multiple collaborators were on a document during an upload.
5598788 to
9aeedd1
Compare
Purpose
During investigations about some users having theirs docs quite slow, we found a method that slow down a lot the users typing on a big documents.
docs/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useUploadFile.tsx
Line 45 in 2864669
Proposal
100vh.