Add functionality to upload folder by dragging to file browser

[Nazeeh21]

References

Add folder upload functionality, fixes #9838

Code changes

in packages/filebrowser/src/listing.ts
I have commented the previous code, which was displaying the dialog box if anyone tries to upload folders, and added the code to upload the folder.

User-facing changes

I haven't added any UI components. I have just removed the error dialog popping up while trying to upload the folder as shown below.

Instead of that, now it will upload the files from the folder.

Backwards-incompatible changes

I haven't implemented any backward-incompatible changes.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[krassowski]

Thank you for working on this. Feel free to remove the commented code :)

I tried it on binder and found that while files are indeed uploaded, the directory structure is lost.

I think that ideally we would want to recreate the folders structures at upload. What do you think?

[krassowski]

We had some short discussion with @blink1073 on the use of experimental API in this PR. I think that we are fine because over 90% of users and all browsers that we support already support this experimental API. However as recommended by MDN docs we should be very careful and defensive when using it; I would suggest checking support first and if it is not there then falling back to the error dialog.

Please let me know if you need help with creating new folders using JupyterLab filebrowser API.

[Nazeeh21]

Thank you for working on this. Feel free to remove the commented code :)

I tried it on binder and found that while files are indeed uploaded, the directory structure is lost.

I think that ideally we would want to recreate the folders structures at upload. What do you think?

Yes, I also feel that retrieving the folder structure is necessary.
I will implement it soon and also I will remove the commented code in the next commit.

[Nazeeh21]

We had some short discussion with @blink1073 on the use of experimental API in this PR. I think that we are fine because over 90% of users and all browsers that we support already support this experimental API. However as recommended by MDN docs we should be very careful and defensive when using it; I would suggest checking support first and if it is not there then falling back to the error dialog.

Please let me know if you need help with creating new folders using JupyterLab filebrowser API.

Yeah sure, I will check their support and will implement accordingly.

I'll try to implement folder creation. In case, I'll need your help I'll surely let you know. Thanks for asking. 😃

[Nazeeh21]

We had some short discussion with @blink1073 on the use of experimental API in this PR. I think that we are fine because over 90% of users and all browsers that we support already support this experimental API. However as recommended by MDN docs we should be very careful and defensive when using it; I would suggest checking support first and if it is not there then falling back to the error dialog.

Please let me know if you need help with creating new folders using JupyterLab filebrowser API.

I got my head around with the createNewDirectory() function from browser.ts. But I am unable to figure out that how to access the FileBrowser class in addDirectory() function in listing.ts to create a new folder and renaming that folder.

Will you please help me with the same?

Also, after creating the folder. I am planning to call this._model.cd('/${folderName}') and upload files in that folder.

Will you please tell me if I am following the correct workflow or not? 😅

[krassowski]

Yes you are going in the right direction. A very quick note that you probably do not want to use FileBrowser.createNewDirectory because in addition to creating a new directory it prompts user to rename it; instead we want to create a new directory and set its name programatically. Following the code I see that it calls manager's newUnititled() method. It seems to me from a quick glance that listing and browser share the same model so you should be able to get the manager from listing's model (something like this._model.manager.newUntitled). You would call the same newUntitled method but then instead of starting rename in browser GUI, trigger programatic rename using managers IDocumentManager.rename.

[Nazeeh21]

Hey @krassowski, I have implemented the addExisting() , rename() and cd() functions here. But it is not renaming the newly created folder and is also not changing the current path. I tried working my head around with that but didn't get any workaround.

I have pushed the code. Will you please help me with the same?

[Nazeeh21]

@krassowski I made the addDirectory function async but that didn't work. From the errors, it seems that it is throwing an error in .cd() function while changing the directory only for the nested folders. I tried finding out the workaround but didn't find anything.

[krassowski]

Ok, this is working now. Still may need some work to work faster. Ideally would have a test case. Still, I think it is an improvement worth considering for 4.3.0.

[krassowski]

We can probably avoid changing the directory by adding a path argument to upload() method of the model. I will take a look another day.

[krassowski]

This looks good to me, but I would appreciate another review

[jtpio]

Maybe something like allUploaded reads better, and would be similar to the naming of other signals?

[krassowski]

Thanks, done!

[JasonWeill]

This looks great! I tested the change locally, and I noticed that it worked successfully. A progress meter would be useful when uploading a large directory or a directory with many large files in it, but that's already covered by #4687, so we shouldn't hold this PR up for that reason.

[krassowski]

Hmm... this made me think that we could already use the progress bar if we moved the logic to the listing model. Actually, I think we should move the logic to model anyways so that it can be easily reused. This would also remove the need for allUploaded because we could listen to uploadChanged signal.

[krassowski]

While I think that progress bar for the entire folder would be useful, I do not have bandwidth before 4.3 freeze. If we decide to move the logic to the model, the allUploaded signal can be easily made into a filter proxy of uploadChanged and deprecated (and it is a protected rather than a public member anyways). The question is only whether the public API in the model should be:

• upload(something: File | FileSystemEntry, path?: str): Promise<Contents.IModel>, or
• uploadEntry(entry: FileSystemEntry, path?: str): Promise<Contents.IModel>, or
• uploadDirectory(directory: FileSystemDirectoryEntry, path?: str): Promise<Contents.IModel>