Skip to content
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

fix for totalProgress when files kept in state #4993

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mok419
Copy link

@mok419 mok419 commented Mar 13, 2024

Mentioned in my FR #4992

This allows not being uploaded files kept in state to not affect the total progress!

Mentioned in my FR transloadit#4992

This allows not being uploaded files kept in state to not affect the total progress!
Copy link
Contributor

Diff output files
diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index c87d245..c72486d 100644
--- a/packages/@uppy/core/lib/Uppy.js
+++ b/packages/@uppy/core/lib/Uppy.js
@@ -727,7 +727,7 @@ export class Uppy {
   calculateTotalProgress() {
     const files = this.getFiles();
     const inProgress = files.filter(file => {
-      return file.progress.uploadStarted || file.progress.preprocess || file.progress.postprocess;
+      return !file.progress.uploadComplete || file.progress.preprocess || file.progress.postprocess;
     });
     if (inProgress.length === 0) {
       this.emit("progress", 0);

@Murderlon
Copy link
Member

I don't think this is something we want to do. It's a bit of an XY problem, this is feedback on the attempted solution rather than the problem. What I think you want is actually some sort of media library, which has been requested often, to browse already uploaded files and upload new files to it. That's a problem which can be solved in many ways, and certainly involves rethinking the UX and the logic we have, not a single line of code.

Furthermore this small change has the potential to break a lot of edge cases.

If that's true, it's better to take a step back and solve this properly. What do you think?

@mok419
Copy link
Author

mok419 commented Mar 14, 2024

I totally agree, I wanted feedback from the team on what I was trying to achieve and how it fits into the current state of the package!

I created this PR as Uppy is suggesting we can keep files in state by setting uploadStarted and uploadCompleted, and this PR fits into that suggestion - as people who use this solution will need to alter the package to keep track of the TotalProgress of a current upload! So it wasn't purely for my use case but also for anyone using the package and keeping files not currently uploading in state!

@mok419
Copy link
Author

mok419 commented Mar 18, 2024

I just had an idea, don't have the chance to test right now, but what if In addition to setting uploadStarted and uploadCompleted true, I set the progress as 100 too? Then totalProgress would think those "in-state" uploads are definitely completed.

Will test when I have a chance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants