-
Notifications
You must be signed in to change notification settings - Fork 53
Sync: Show file upload progress in the UI #2332
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
Sync: Show file upload progress in the UI #2332
Conversation
| <div className="a8c-body-small flex items-center gap-0.5"> | ||
| <Icon icon={ info } size={ 16 } /> | ||
| { pushState.status.message } | ||
| { getPushUploadMessage( pushState.status.message, uploadPercentage ) } |
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 not sure if this is best practice but this is best I could come up with. Let me know if you have any better idea to implement this in other way.
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! Could also be memoized with useMemo, but fine as-is.
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 makes sense. I have updated in 9d16c42
📊 Performance Test ResultsComparing 9d16c42 vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
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.
| return key === 'uploadingPaused'; | ||
| }; | ||
|
|
||
| const isKeyUploading = useCallback( ( key: PushStateProgressInfo[ 'key' ] | undefined ) => { |
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.
All of the isKeyXXX, getPushUploadPercentage, and getPushUploadMessage functions could be defined outside the hook, since they're pure functions with no dependencies on hook state. This provides stable references without the useCallback overhead.
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.
Interesting. Let me refactor them.
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.
| <div className="a8c-body-small flex items-center gap-0.5"> | ||
| <Icon icon={ info } size={ 16 } /> | ||
| { pushState.status.message } | ||
| { getPushUploadMessage( pushState.status.message, uploadPercentage ) } |
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! Could also be memoized with useMemo, but fine as-is.
- Add sync-upload-progress IPC event to send upload progress from main process - Update Tus onProgress callback to send progress percentage (0-100%) - Store raw upload progress in SyncPushState for display - Display upload percentage next to 'Uploading Studio site…' message - Progress updates in real-time as chunks are uploaded via Tus protocol Fixes STU-1144
- Move upload progress mapping logic to useSyncStatesProgressInfo hook - Add getPushUploadMessage helper for i18n-formatted messages - Add isKeyUploading helper function - Update message format to show percentage in parentheses: 'Uploading site (98%)…' - Follow i18n best practices using sprintf for formatted messages - Centralize all progress-related calculations in one place
e41d130 to
9d16c42
Compare
bcotrim
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.
Thanks for making the changes @gavande1
LGTM 👍
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.
This LGTM and work well! 👍
CleanShot.2026-01-06.at.10.13.05.mp4
Just a small heads up that I've been working on refactoring the pull/push states in #2037. I think we can proceed with merging this and it will be fairly straightforward to resolve any conflicts.

Related
Fixes STU-1144
Summary
This PR implements file upload progress tracking for sync operations, showing the upload percentage (0-100%) next to the "Uploading Studio site…" message in the UI.
Changes
Testing