#1193 - optimizing the uploader layout & taking user configured sorting into account

[AndyScherzinger]

For the detailed discussion please see #1193, see attached the screenshots for both sortings from my local tests. Works fine switching the sorting in app and then launching the uploader screen via another app.

This branch is based on master

Beware of certain "details":

• The buttons are still white since the primary action coloring is part of the material_buttons branch
• The list is styled according to @jancborchardt's latests wishes (background white, devider #eee), as it is done for all other lists on the material_buttons branch
• The add folder action is shown on the action bar which will be hidden in the future, as seen on the material_fab branch since with the introduction of the fab the general guideline has been to hide non-primary actions in the actionbar in the overflow menu
• since the date calculation/visualization is now used in two places I refactored the code accordingly (within this PR)

PR is ready to review, so pinging @tobiasKaminsky @LukeOwncloud @masensio @davivel @jancborchardt for review and integration within the beta branch :)

@jancborchardt Little detail: The title is a re-use of the choose-text which I changed for english but will be "wrong"/old before the branch is merged into master and transifex translations for the other languages come in. Just letting you know :)

---

BUGS / IMPROVEMENTS:

• Name of the app in uploader view #1193 - optimizing the uploader layout & taking user configured sorting into account #1277 (comment) [FIXED]
• Kind of view in uploader view #1193 - optimizing the uploader layout & taking user configured sorting into account #1277 (comment) [WON`T FIX]
• Divergence between internal and external uploaders #1193 - optimizing the uploader layout & taking user configured sorting into account #1277 (comment) [FIXED]

[tobiasKaminsky]

Code looks fine 👍
@AndyScherzinger can you merge this into beta and write about it in Changelog.md?

[jancborchardt]

The upload button lost its primary class (blue color) somehow? Otherwise it looks good.

[AndyScherzinger]

see initial comment :) The primary coloring cannot be applied since that is part of the material buttons PR dating back to August which hasn't been merged yet... :'(

[jancborchardt]

Ah ok, sorry for not reading everything of that. ;) Looks fine for me then.

@davivel @masensio @rperezb please review.

[AndyScherzinger]

here is the screenshot of the upload screen on the beta branch :)

[AndyScherzinger]

PR is ready to review, so pinging @tobiasKaminsky @LukeOwncloud @masensio @davivel

[tobiasKaminsky]

Looks good to me.
Merging it into beta.

[tobiasKaminsky]

Uh. I had already merged this ;)

[AndyScherzinger]

@jancborchardt any help? You asked for this via #1193 :) (not wanting to push my PRs into the releases knowing that @tobiasKaminsky also has a lot of unmerged PRs and the core team is working on customer features).
Still without wanting to sound too bragging/demanding we really need to get back to merging community PRs. 😢 Besides I really hope that Tobias' beta initiative will help pushing the community PRs forward to have fast merges in the future 👍

[AndyScherzinger]

@rperezb @davivel @masensio pinging you since this PR isn't yet assigned to any milestone (not even backlog), is rather small and fixed the issue #1193 opened by @jancborchardt 😃

[jancborchardt]

@AndyScherzinger I think design-wise it looks cool! 👍 :) What’s behind the 3-dot-menu? I assume »Create new folder« and »Sort order«?

[AndyScherzinger]

@jancborchardt exactly :) At least for this PR. If I am not mistaken the create folder menu item might have been removed in some other issue (not sure though...). At least it is missing in the beta build ( @tobiasKaminsky )

[jancborchardt]

We should bring that back though (maybe not in this PR), cause you might wanna add a folder to put the new files in.

[jancborchardt]

(And I assume it was removed because it was moved to the FAB. In this view the FAB would be confusing though.)

[tobiasKaminsky]

Yes. I think this is the cause.
So, this should be in your PRs @AndyScherzinger ?

[AndyScherzinger]

Exactly, I will update the fab branch accordingly.

[AndyScherzinger]

@jancborchardt just fyi, I just updated to the latest master and realized that the answer above with the menu isn't correct. The menu in the upload folder chooser only contains the create folder, nothing else.

[tobiasKaminsky]

Then we could use the FAB also there?

[AndyScherzinger]

I don't think so, but we can add it. At the time of implementing the we decided to explicitly not show it.
So question would be if there are any other menu items that should be present here. Even though this would be an addition since the implementation for any other actions isn't present as of today.
So that could then be done in an other PR.

... Trying to not do the usual mega PR thing... ;)

So I would leave the decision to Jan on adding a Fab, but rather wouldn't since it is a folder chooser, so the main action is the OK button.

[tobiasKaminsky]

You are right. But it would "just" moving the same function from three dot menu to FAB.
Maybe a separate PR is the better way.
Coming in the future in the uploader screen in the three dot menu would be "select all". If we would move the "create new folder" then we could show the "select all" button directly without a menu.

[jesmrec]

In the #1193 you have discussed about "matching" both uploaders view (internal and external), because of that we have the current PR.

Summarizing, we have the current behaviour in master and feature branches:

Branch	Internal Uploader	External uploader
Master	Folders do not show info Files show date and size	Folders do not show info Files do not show info
1193_uploader_layout	Folders do not show info Files show date and size	Folders show date Files show date

Checking the chart can be noticed that you are really showing more info (2nd row contains more info as 1st row in the second column), but this info diverges more from the internal to the external uploader:

• Folders show now the date in external, but in internal do not (should they?, --out of scope--)
• Files show now the date in external, but internal show date + size (here it is closer, but not equal)

So, we have more info, but more different. What do you think about? :)

[AndyScherzinger]

IMHO Folders and Files should show the same info. Since this PR is about sorting I think we should display the information which would then also match the main view as in

• date + size for files
• date for folders

any comments @rperezb @davivel @jancborchardt

[AndyScherzinger]

@davivel please review again
@jesmrec please test again

my latest code change now does the following: the main file list, the internal uploader, the external uploader all look the same layout wise and they all render the same data (folders: last modification, files size, last modification).

Hope you like it this way 😃

[davivel]

Reviewed, 👍

Great work here, pals :)

[jesmrec]

@AndyScherzinger

i love this way 👍 👍

Approved

[davivel]

@AndyScherzinger , could you please rebase and merge? Just to prevent I appear as the comitter...

Thanks a lot!

[AndyScherzinger]

You're very welcome! Rebased, just waiting for travis to be able to merge then 🎉
Great collaboration and pace 👍