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

Allow whole folders to be uploaded to the server #4505

Closed
wants to merge 1 commit into from

Conversation

robb4
Copy link

@robb4 robb4 commented Sep 14, 2019

Long press the folder to select it
Refs: #184

Long press the folder to select it
Refs: nextcloud#184

Signed-off-by: Robert Dumitrescu <robert@dumitrescu.pm>
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/10924.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@tobiasKaminsky
Copy link
Member

2019-09-19-223332

Instead of adding a non-visible long press action, I would simply use the multiple choice checkbox for this.

@robb4
Copy link
Author

robb4 commented Sep 20, 2019

Hmm, your screeshot looks a bit off, this is how it looks for me:
screenshot_2019-09-20_07-42-36_584659138

Currently, for files, the checkbox gets activated by a single tap, but a tap on a folder navigates to it's content. It would make a bit more sense UX-wise to select both types on a tap, but that would interfere with the current behaviour. Is it acceptable to change the navigation option to a long tap?

@tobiasKaminsky
Copy link
Member

Sorry, I somehow was on the wrong branch.
Adding long press is something no user will directly know/understand, so I think we can omit it.
Having a checkbox for a folder should be enough @jancborchardt?

@tobiasKaminsky
Copy link
Member

@robb4 thanks for contributing to this long wished feature 🎉

@tobiasKaminsky
Copy link
Member

I created such a folder structure

  • 1
  • 1/2
  • 1/2a
  • 1/2/test.pdf

and select 1 to be uploaded, but "only":

  • 1/2/test.pdf was uploaded
    Empty 2a was not created.
    I guess it as we only upload files.
    I fear that people expect that also empty folders are created, which then unfortunately complicates it a lot…

@robb4
Copy link
Author

robb4 commented Sep 20, 2019

@tobiasKaminsky it does indeed only upload files, and it does not create empty folders. I'll take a look over this use-case when I'll have some free time.

@jancborchardt
Copy link
Member

Sorry for the late reply – yes the checkbox seems good to select folders! 👍

@robb4 any update on the other parts? :)

@robb4
Copy link
Author

robb4 commented Oct 12, 2019

@jancborchardt Sadly I haven't had the time to work on this lately.
I was initially hoping that creating empty folders too would be a quick and easy fix, but at a closer look it showed to be a bit more problematic.
I tried piggybacking off the 'Create Folder' action available in the same activity, but that got messy quickly, so last time I was investigating different solutions to this. I could probably use some hints from someone who's more familiar to the architecture :)
I hope to be able to get back to this in the following weeks.

@kilves
Copy link

kilves commented Nov 17, 2019

@robb4 Are you still working on this? I accidently started this issue without even noticing that it had been under development for a few months already. I also completely missed the parameter on requester.uploadNewFile() that allows you to auto-create folders, so I ended up implementing my own folder creation. My redundant-but-useful solution might be of some help: kilves@0297601 the folder creation code here could probably be adapted to also create empty folders with just a few lines of extra code.

There's also a $300 bounty on this in bountysource if you're interested in splitting it: https://www.bountysource.com/issues/36543232-upload-whole-folder

@robb4
Copy link
Author

robb4 commented Nov 17, 2019

@kilves No, I haven't really worked on the core issue with my PR (the empty folders part), I only have a few local changes like not using long press to select and some other messed up code written while investigating.
To be honest, if your solution already ticks all the requirements go for it and raise a PR. If it's not complete and you think I can help you with something then sure, drop me a line on IRC or something and we can collaborate, but from what I've looked over your code I don't believe there's any benefit of merging the two solutions together.

@kilves kilves mentioned this pull request Nov 19, 2019
@AndyScherzinger
Copy link
Member

superseeded by #7877

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.

6 participants