Skip to content

Multi file upload#861

Merged
notsidney merged 11 commits intodevelopfrom
multi-file-upload
Nov 17, 2022
Merged

Multi file upload#861
notsidney merged 11 commits intodevelopfrom
multi-file-upload

Conversation

@shamsmosowi
Copy link
Contributor

No description provided.

@shamsmosowi shamsmosowi requested a review from notsidney October 11, 2022 22:06
@vercel
Copy link

vercel bot commented Oct 11, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
rowy-os ✅ Ready (Inspect) Visit Preview Nov 16, 2022 at 8:17AM (UTC)

onChange(newValue);
files: acceptedFiles,
onComplete: (newUploads) => {
onChange(arrayUnion(newUploads));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can’t use arrayUnion here because this goes in the local state, which doesn’t handle this Firestore-specific value. If a row is local (e.g. created with random ID), when the files are uploaded, the cell value appears blank.

@htuerker
Copy link
Contributor

Screen.Recording.2022-11-15.at.07.22.58.mov
Screen.Recording.2022-11-15.at.08.21.30.mov

Copy link
Contributor

@notsidney notsidney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@htuerker just some minor comments. Let me know if you want to make any changes before I merge it into the new table branch.

Comment on lines +76 to +77
return new Promise((resolve) =>
files.forEach((file) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m confused with how this is working. You create one promise when multiple files are uploaded, and it seems to resolve the promise when only one of those files are uploaded or fails to upload. Am I missing something here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It pushes a value into uploads or failures when onComplete/onError independently for each upload process, this promise only resolves once the total length of those matches with the input files length.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed the isCompleted() calls. An alternative solution is to create an array of promises for each file, which resolve when the file is added to uploads or failures. Then, create a new promise using Promise.all( <array of promises> ) and wait for that to resolve. Your solution works fine though.

@notsidney notsidney merged commit bcf6dfc into develop Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants