Optimizing the uploader layout & taking user configured sorting into account

[AndyScherzinger]

NC version of @jancborchardt's original oC request 1193:

When uploading content from another app (like the Gallery), the uploader view looks strangely different from the normal file list.

It should have the same layout, with file/folder info below the folder names, and also the same sort order if one chose to sort by modified date.

pinging @tobiasKaminsky @przybylski for the 2-thumbs-up code review

could be put into a post 1.0.0 release since it is a rather small change and easy to test (already tested by me)

[przybylski]

To me this class name is super confusing, can you rename it to something like PreferenceHelpers ?

[przybylski]

Also I don't understand why this is in db package

[AndyScherzinger]

left this one open since it is a structural change

[AndyScherzinger]

Very thorough review @przybylski thank you for that! 👍

I would like to start a discussion on this including @tobiasKaminsky in this. All the review comments are very legit and push for better code. The changes I made have been done with minimal changes in mind to be necessary to add the functionality but stay with the given/oC code structure to keep merges easy.

It's tricky since easy merges mean we are stuck with the code as it is when we forked which might slow us down here and there. Diverge right at the start will make merge rather hard and might lead to errors down the road due to mistakes made while manually merging/adapting changes.

Looking forward to your comments :)
...don't hit me 😉

[przybylski]

So let's wait what will @tobiasKaminsky say

[AndyScherzinger]

Sounds good to me. I am fine either way though but it should be a decision we all agree on :)

[tobiasKaminsky]

Tricky one...
I would do a "middle way". In order to get all pending >50 PRs as quick as possible in Nextcloud we should stay very near to the original code.
After that we can and should refactor as much as we want. Of course this will make backports very time consumption...

[AndyScherzinger]

Sounds good, thus I would propose the following steps:

1. I fix the review comments which do not have a larger impact on the code structure
2. We test and merge (I already tested it and it works)
3. I open up an issue referring to this PR for refactoring aspects to be done after we are through which a larger part of the open PRs

Due to the possibility of differences in pace and future features the code base will diverge at some point anyways, but yes which should aim to get PRs merged and thus old branches removed and then move on from there.

Sounds good to you guys? @nextcloud/android developers epsecially @przybylski and @tobiasKaminsky ?

[tobiasKaminsky]

The plan sounds fine! (I need to avoid everything that l g t m can think I like this PR ;))

[AndyScherzinger]

Well we could remove the thumbs up pattern so only shipit and L g M T would trigger the approval 😃

[AndyScherzinger]

Please review again. @tobiasKaminsky and @przybylski I incorporated all non-structural changes and also fixed some more layouting/data-display stuff so now all uploaders and the standard file list look exactly the same

[przybylski]

Wouldn't it be better to have one text view and format string when inserting?

[AndyScherzinger]

@przybylski I vote no since this would be the next change that doesn't add additional value but let's the code diverge from oC

[AndyScherzinger]

opened an issue for the dimensions housekeeping: #12 to move stuff like this to dims.xml 😃

[AndyScherzinger]

So are you fine with the changes @przybylski ?

[przybylski]

aside some structural changes LGTM

[AndyScherzinger]

Thanks a lot! Besides that looking forward to @tobiasKaminsky feedback especially on the matter on how (and when) to deal with technical debt and how to compile a list of PRs we want to integrate before we start larger refactorings.

[AndyScherzinger]

Sidenote... just realised that I (as in the person who opened the PR) can't even have a 👍 in a comment at all without that being tracked as a positive code review feedback... 😣

[tobiasKaminsky]

@AndyScherzinger of course it is. Just like LGTM needs it 👍

[AndyScherzinger]

Sweet, so waiting for the build to finish and then I will merge :)