Skip to content

Conversation

@AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented Aug 8, 2016

for initial discussion please see #195

Open issue:

  • quota string formatting. For now I went with %1$s of %2$s used which could re-use the given formatter for (might need some work or a slightly different string) so %1$s and %2$s would be the sizes with the units calculated like the one in the file lists e.g. "1MB of 3,0GB used", another idea (like dropbox) would be to mix: "10% of 3,0GB used" - both have their advantages and disadvantages.

DONE

  • coloring of the progress bar: "just blue" or turning yellow-ish and red-ish when hitting certain thresholds like 85% and 95%? And if so which yellow/red colors should be used.

As a sidenote, for now I had to add a "invisible" menu item for the bottom padding since I cannot customize the layouting of the menu-list itself which is imho okay until AppCompat provides more options to customize the drawer itself.

cc @tobiasKaminsky for further development and @jancborchardt regarding the quota string/format and colors to be used.

device-2016-08-08-172117

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 1.4.0 milestone Aug 8, 2016
@AndyScherzinger AndyScherzinger added the needs info Waiting for info from user(s). Issues with this label will auto-stale. label Aug 8, 2016
@AndyScherzinger
Copy link
Member Author

@tobiasKaminsky please see c264f38#diff-154af4ff2566e264dcca122072d20ae7R507 for an easy starting point, for now I have marked them private since I am not yet sure if everything could be handled within the DrawerActivity.

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 9, 2016

@tobiasKaminsky UI part is done, see below. Can you help me out with the caching part and also check the code if I implemented the server calls correctly (first time for me in this project)... PLEASEEE 😸
device-2016-08-09-190834

Weird thing I realized is that as soon as I activated the 5 GB for the test account (tiger), my personal account even though it is set to unlimited shows a quota of 1 TB ?! Id this on purpose? Not sure who to ask to pinging @LukasReschke @nickvergessen @MorrisJobke and @rullzer ... Sorry guys for the noise.

@tobiasKaminsky
Copy link
Member

I'll do it tomorrow, wednesday.
Great work!

@AndyScherzinger
Copy link
Member Author

I implemented a threshold based coloring of the progress bar (quota) with two thresholds

  • INFO (blue): < 90
  • WARNING (yellow): >= 90 and < 95
  • CRITICAL (red): >= 95

(hard coded demo screenshots, so please ignore the numbers shown)
color_infolevel

@AndyScherzinger
Copy link
Member Author

@jancborchardt is this fine with you color and threshold wise?

@tobiasKaminsky
Copy link
Member

+1 for color.
Great

@AndyScherzinger
Copy link
Member Author

Thanks @tobiasKaminsky :) Did you already have a chance to to look into the caching aspect?

@jancborchardt
Copy link
Member

jancborchardt commented Aug 12, 2016

Awesome! Looks good. I’d only change 2 things:

  • wording/values: use the same unit for both current and max value, likely MB or GB. Also do not use decimal values for MB or KB as it’s too little anyway.
  • remove the darker grey background, it’s not needed.

@AndyScherzinger
Copy link
Member Author

@jancborchardt @nextcloud/designers Thanks for the Feedback, just a minor issue to clarify:

wording/values: use the same unit for both current and max value, likely MB or GB. Also do not use decimal values for MB or KB as it’s too little anyway.

This is only for the quota right? For formatting I used the implementation that we use to display the individual file size for each file in the list views, which shows 1 decimal for MB, see:
https://github.com/nextcloud/android/blob/quotaDisplay/src/com/owncloud/android/utils/DisplayUtils.java#L69

So should we keep 1 decimal for MB for files but none for MB displaying quota or remove the MB decimal everywhere? (Bytes and KBytes don't have a decimal anywhere)

@tobiasKaminsky
Copy link
Member

@AndyScherzinger thinking about caching I think it is not necessary.
The overhead is so small in comparison to a refresh/listing of a directory and the risk is very high to show a deprecated value.
So I would leave it the way it is now. Hope you are okay with this?

@tobiasKaminsky
Copy link
Member

Ping me, when I should do a CR

@AndyScherzinger
Copy link
Member Author

@tobiasKaminsky you can start right away. Only remaining changes I'll do are:

  • remove grey background color
  • remove decimal for MB (probably a separate beautifier method like the one we have simply with another scales-Array :)

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 14, 2016

@jancborchardt @nextcloud/designers

wording/values: use the same unit for both current and max value, likely MB or GB. Also do not use decimal values for MB or KB as it’s too little anyway.

That doesn't make sense! If you used 2MB and have a Limit of 1GB you wouldn't want to display 0,002GB of 5GB used even if you do it the other way round it looks strange to me 2 MB of 5120 MB used (seems even worse to me since it is even more technical looking). Thus I went for no digits for B,KB,MB and 1 for GB and TB but strongly advice to drop that requirement of same unit. Only other smart thing I can think of is going for percentages like 2% of 1GB used or simply leave it the way it is.

@AndyScherzinger
Copy link
Member Author

device-2016-08-14-201710

@AndyScherzinger
Copy link
Member Author

@nextcloud/designers here is a little problem imho with the white background for the quota...

device-2016-08-14-202221

Now the quota seems to be part of the menu (which it is not) and the user might not get that he can scroll (see right image)..

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 18, 2016

Personally I would be fine with just going with

  • percent-used% of total-space unit

While as a rule, we could "glue" it to the coloring rules

  • INFO (blue): < 90 - scale 0 --> e.g. 89% of 7,5 GB
  • WARNING (yellow): >= 90 and < 95 - scale 0 -->91% of 7,5 GB
  • CRITICAL (red): >= 95 - scale 1 -->96,2% of 7,5 GB

I understand the issue with scale 2 for large total-space scenarios but would still say 99,x is good enough since you are already very close to your limit. So if you have 1TB as total space and 99,9% are in use you do have a problem already simply since you are a power user storage wise...

So how about my coloring / scale approach, it might not be the perfect solution (if there is any) but it is better than not having anything 😄

@jancborchardt
Copy link
Member

We don’t need to add % because the bar already signifies that.

Regarding the white background I think it’s fine @AndyScherzinger. :) If it’s really a problem, maybe it’s possible to make the background slightly transparent so it’s clear there’s items below?

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 18, 2016

@jancborchardt the background shouldn't be a problem. The drawer and the scroll-able menu can be considered "standard behavior" since all Google apps and any other app implementing a drawer menu I can think of supports scrolling, so I think the user is expecting this anyways :)
As for the quota formatting, we can than just keep it like this and wait for any user feedback after the release.

I'll postpone this PR until 9.0.54 has been released. To be done then:

  • update the library to also retrieve the quota on/off flag
  • show/hide the quota UI elements for unlimited quota configuration

@jancborchardt
Copy link
Member

Sounds good yes :)

@Bugsbane
Copy link
Member

We don’t need to add % because the bar already signifies that.

Good point.

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 31, 2016

@jancborchardt one issue to be decided. How should the app behave for Servers not supporting the new Quota (NC < 9.0.54 and oC - both not detecting unlimited quota) ?

  • always show a quota, which for unlimited just means showing the total size
  • never show a quota since "unlimited" cannot be detected

I am fine either way. I just need to know and since I am already running Nc10 this isn't a matter for my personal scenarios. -

Edit: Actual implementation is:
If unlimited can't be detected (this fact is propagated) than the total size will be used. If the quota is unlimited, then no quota will be shown.

Second issue where we don't agree: wording/values: use the same unit for both current and max value, likely MB or GB

--> I say let's keep it the way it is with appropriate, calculated size-units for used/total, you gave the best example yourself with "total" most likely being "GB".

  • total 20 GB, would due to the "GB" be rendered without decimals: 20 GB
  • used could be (see screenshots) 698 KB bumping it to GB would mean: 0,0065 GB
    • would this then be "GB no decimals": 0 GB
    • or kept this way: 0,00066 GB
    • or go nuts to loose decimals: 65e-5 GB 😉

Both don't make sense at all ever, with a factor of 1024 per size used<>total shouldn't be size-unit equal !

Another issue I just realized is by having implemented your request to have no decimals for MB the Android client now differs from the server.

  • Server showing my quota: 9.6 MB / 19.9 GB
  • Client showing my Quota: 10 MB / 19.9 GB

@AndyScherzinger
Copy link
Member Author

@jancborchardt PLEASE take a look at my last comment #204 (comment) - development wise this PR is implemented, tested and code reviewed so this one is ready to go for the next release!

@AndyScherzinger
Copy link
Member Author

added to beta branch #169

@jancborchardt
Copy link
Member

@AndyScherzinger

If unlimited can't be detected (this fact is propagated) than the total size will be used. If the quota is unlimited, then no quota will be shown.

Sounds good I guess!

  • appropriate units are fine instead of forcing the same, you are right
  • ah right, so only no decimals for kB, sorry. One decimal for MB and GB is fine.

👍 :)

@AndyScherzinger AndyScherzinger removed the needs info Waiting for info from user(s). Issues with this label will auto-stale. label Sep 1, 2016
@AndyScherzinger AndyScherzinger merged commit 303b05a into master Sep 1, 2016
@AndyScherzinger AndyScherzinger deleted the quotaDisplay branch September 1, 2016 09:03
@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Sep 1, 2016

...aaaand merged to master to be included in the next release 🚀, also put to the beta branch with the latest changes. Thanks everyone! 👏

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.

7 participants