owncloud / android

:phone: The ownCloud Android App
GNU General Public License v2.0
3.84k stars 3.06k forks source link

Confusing: Size is too close to date #1232

Closed tobiasKaminsky closed 9 years ago

tobiasKaminsky commented 9 years ago

date At least for me I read "27, 2 Mb" and not "Jun 27" and "2 Mb". While I understand the approach to have the Filesize left aligned, I guess this not perfect...

Ps.: Is this beta related? I do not know from which PR this is...

AndyScherzinger commented 9 years ago

From mine (button or fab, would have to check), this has been an explicit requirement by @jancborchardt

jancborchardt commented 9 years ago

Good observation – we should put the size first, and then the date. That has the added advantage that the numbers are more or less aligned and comparable. (Whereas the date string is highly variable in length, especially the relative date.)

tobiasKaminsky commented 9 years ago

@AndyScherzinger can you edit and commit this in your original PR? And then ping me, then I can update beta.

AndyScherzinger commented 9 years ago

Sure, no problem :)

AndyScherzinger commented 9 years ago

@tobiasKaminsky flipped the infos on https://github.com/owncloud/android/tree/material_buttons

jancborchardt commented 9 years ago

Btw what’s the color of the divider line between files? It should be #eee (very light grey, and only one pixel thick) as in the web interface, but seems darker than that.

AndyScherzinger commented 9 years ago

I checked, it has been #F0F0F0 except for the drawer where I already used #eee Just fixed it with the latest commit https://github.com/owncloud/android/commit/efdcb979632effba0b62f3422d5ff8f9974087b6 to the material_buttons branch

jancborchardt commented 9 years ago

Cool, thanks! Maybe if even #eee (nearly the same as #f0f0f0) is so dark, we might need to change it to something lighter such as #f8f8f8. How does that look @AndyScherzinger?

AndyScherzinger commented 9 years ago

Your decision @jancborchardt ;)

eee

device-2015-11-02-142327

f8f8f8

device-2015-11-02-142533

The lines "look" thinner with the lighter color which they actually aren't the dividers size is always 1dp (not 1px!) since we never make your of px but always use dp :)

AndyScherzinger commented 9 years ago

@tobiasKaminsky I did the change and merged it into the beta branch already :)

jancborchardt commented 9 years ago

f8f8f8 is definitely better! :)

Also, what is this extra bar of blue below the header? Seems strange, no?

And as well (and probably again) – I think it’s confusing to have a refresh button in the header. There’s pull to refresh and while I know that one is for refreshing the whole account and one for the current folder, we should fix this properly. Otherwise people will use the more present option (the button) and then always refresh the whole account anyway. cc @davivel @masensio

AndyScherzinger commented 9 years ago

@jancborchardt

jancborchardt commented 9 years ago

the blue bar below the bar is the progress bar

Ah ok – but it only shows when refreshing or loading, right?

the sync icon is shown on the bar (atm in the beta) since with the fab there are only 2-3 actions in total.

Ok – but all of these actions (refresh, switch sorting, switch view) are not very important, often used actions. I would hide them all in a dot-dot-dot menu. What do you think @tobiasKaminsky @AndyScherzinger? That way it’s more clean and focus stays on what’s important.

AndyScherzinger commented 9 years ago

the blue bar below the bar is the progress bar

Ah ok – but it only shows when refreshing or loading, right?

It is always shown since the bar itself is always present (progress bar) which is configured as being indeterminate and thus is only animated when triggered, but the element is always visible (blue line) else whenever we would make the element visible the the underlying list view would "jump". Unfortunately I haven't found the reason for the progress bar being slightly darker...yet :(

hide the actions

Both variants are fine to me:

looking at the actions present after adding the fab I guess all other actions are secondary ones and thus can be hidden on the overflow menu (dot-dot-dot).

jancborchardt commented 9 years ago

but the element is always visible (blue line) else whenever we would make the element visible the the underlying list view would "jump".

Ah got it. Ok, so yeah either it should look as if it’s part of the header, or it should be transparent.

looking at the actions present after adding the fab I guess all other actions are secondary ones and thus can be hidden on the overflow menu (dot-dot-dot).

Yup – less cluttered UI takes precedence here because the »one click less« doesn’t really apply to less-used secondary actions. Cause in that screen, showing them directly would call to much attention.

AndyScherzinger commented 9 years ago

Header

I will have a look, but that might be quite a challenge since the absolute correct way to do this would be to use a toolbar since that is an element where you are able and allowed to influence its content, like adding a progress bar. Unfortunately I could get it to work since the fragment fire events like crazy which than made the toolbar's back/home navigation go nuts and thus reverting back to the action bar. I am not to familiar with fragments and the complete codebase but that is how I ended up implemetning it (so atm I can't really do anything about it :( ) - not an excuse but an explanation though.If anybody is firm with the fragments' implementation and their reporting back to the activity please let me know so we can move to toolbar on a feature branch :) - which I would love btw since we could then implement the "hide toolbar when scrolling down" behavior :grin:

jancborchardt commented 9 years ago

Ok, no worries :) The loading bar is a minor detail I would say. But putting the icons in the dot-dot-dot menu are important, and I guess way easier to do.

AndyScherzinger commented 9 years ago

That is easy, yes :) @tobiasKaminsky can you enforce the overflow in your feature branches as described by @jancborchardt with the fab button it is basically everything to the overflow menu

The progress bar is a detail sure, but having the toolbar would be really nice but for that I would need a joint venture with at least @davivel @tobiasKaminsky @masensio

AndyScherzinger commented 9 years ago

Photo finish for @jancborchardt - @tobiasKaminsky I fixed the menu on the beta branch already, see https://github.com/owncloud/android/commit/01115a1cd50362a22924845ddc72c74467ad5d10 , also added to the material_fab branch (see https://github.com/owncloud/android/commit/72b330d25b673a702ebdcfcac8f2884fe214ecfd)

screenshot showing actual beta with menu overflown and the flipped size/mod-date: device-2015-11-02-180210

tobiasKaminsky commented 9 years ago

@jancborchardt Please open the next time a new issue if you have additional ideas/wishes as this issue now contains three different topics and is in the future hard to find or distinguish.

Regarding hiding the menu actions: As I am using sorting and "switch layout" very often I do not want to hide them, but yes the UI is now much cleaner.

tobiasKaminsky commented 9 years ago

@AndyScherzinger please do not put changes directly into beta branch. The purpose of the beta version is to prove correctly and test the PRs, we want to merge into master. With sideloading changes we can get new errors/problems that might be not in master, when merging only the PRs.

AndyScherzinger commented 9 years ago

Sorry about that, from now on all changes will be done using separate branches for them. Hope it did not cause any trouble.

jancborchardt commented 9 years ago

Please open the next time a new issue if you have additional ideas/wishes as this issue now contains three different topics and is in the future hard to find or distinguish.

@tobiasKaminsky yeah, sorry about that. :> Sometimes I’m guilty of asking for too much unrelated stuff.

Regarding hiding the menu actions: As I am using sorting and "switch layout" very often I do not want to hide them, but yes the UI is now much cleaner.

However, we should make this as intelligent as possible, and even though you use the layout switcher often, it is still way less important than everything else on the screen. (Which is the title, list of files, back button, and creating new files.) As always: If everything is important, nothing is. ;) And that is especially true with the limited space on mobile. So we need to really really make sure to always only show the relevant stuff and, while still keeping it accessible, hide the less important things.

tobiasKaminsky commented 9 years ago

Yeah :) I know you are right from a ui expert view, but my personal view is still different. But this is ok as we cannot make a ui that fits all needs.

beta3 This now looks strange ;)

AndyScherzinger commented 9 years ago

Ah, I'll fix that on the material_buttons branch. The code has set the size invisible which simple makes the UI element invisible but still takes up the space. So since we flipped size and date we need to use visibility.gone instead of invisible :)

AndyScherzinger commented 9 years ago

Fixed and merged, see https://github.com/owncloud/android/commit/4d33ecf15e9a4fa22e1e49e563793704c7598a3f

AndyScherzinger commented 9 years ago

Original PR against master is #1209

tobiasKaminsky commented 9 years ago

Looks good in beta (will available this afternoon), therefore closing it.

AndyScherzinger commented 8 years ago

assigned it to 1.9.1 and removed beta since it has been merged to master and will be shipped with v1.9.1.