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

Add quota to the files view #5305

Merged
merged 11 commits into from
Jun 13, 2017
Merged

Add quota to the files view #5305

merged 11 commits into from
Jun 13, 2017

Conversation

nickvergessen
Copy link
Member

bildschirmfoto vom 2017-06-08 16-04-09

cc @jancborchardt apply beautification 😸

@Dennis1993
Copy link
Contributor

Dennis1993 commented Jun 8, 2017

That's a very good idea. I think we can remove the quota overview in the personal settings and move it to the files overview.

My idea is to set it a little bit down and not of the top of the navigation. Here an example. Whats your opinion?

Normal state (free space available)
normalstate

Full state (free space is low)
fullstate

@MorrisJobke
Copy link
Member

My idea is to set it a little bit down and not of the top of the navigation. Here an example. Whats your opinion?

That was also our idea to put in down there.

@MariusBluem
Copy link
Member

MariusBluem commented Jun 8, 2017

I think we can remove the quota overview in the personal settings

👎 ... You'll get a nice overview there, and we need this bar for the quota-buttons (apps/external).

@Dennis1993
Copy link
Contributor

@MariusBluem oh okay. I don't know that, I don't use external storage :)

@codecov
Copy link

codecov bot commented Jun 9, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@2f7f6e2). Click here to learn what that means.
The diff coverage is 40.9%.

@@            Coverage Diff            @@
##             master    #5305   +/-   ##
=========================================
  Coverage          ?   54.14%           
  Complexity        ?    22313           
=========================================
  Files             ?     1381           
  Lines             ?    85470           
  Branches          ?     1325           
=========================================
  Hits              ?    46275           
  Misses            ?    39195           
  Partials          ?        0
Impacted Files Coverage Δ Complexity Δ
apps/files/templates/index.php 0% <ø> (ø) 0 <0> (?)
apps/files/templates/appnavigation.php 0% <0%> (ø) 0 <0> (?)
apps/files/lib/Controller/ViewController.php 76.31% <81.81%> (ø) 18 <0> (?)

@jancborchardt
Copy link
Member

jancborchardt commented Jun 9, 2017

There you go:
screenshot from 2017-06-09 15-40-08

  • Can we move it to the bottom, above »Deleted files«? Otherwise this really takes too much attention away from the important things. Same for the mobile apps.
  • The tooltip with the percentage can have the translation removed in the code. Couldn’t figure out how to do that last part.
  • The link needs to be linked to the actual place where it should go. For the normal release, it should go to the personal settings.
  • For the core version we should put a switch in which disables the »Do you need more space?« sentence, right?
  • If we have a CSS/canvas expert here we could even make the icon reflect the actual space usage. ;) cc @nextcloud/designers @Espina2

@MorrisJobke
Copy link
Member

Can we move it to the bottom, above »Deleted files«? Otherwise this really takes too much attention away from the important things. Same for the mobile apps.

It is only in the branded apps on the top. :/

@MorrisJobke
Copy link
Member

The link needs to be linked to the actual place where it should go. For the normal release, it should go to the personal settings.

Why is it there for the normal version? This should be the "quota" type link from the external sites app and if this is not configured should not show up at all.

@MorrisJobke
Copy link
Member

For the core version we should put a switch in which disables the »Do you need more space?« sentence, right?

See comment above

@jancborchardt
Copy link
Member

Ok, so TL;DR what to change here before we can merge? ;)

@nickvergessen
Copy link
Member Author

I will have a look to bring the externals app link into this

@MariusBluem
Copy link
Member

I will have a look to bring the externals app link into this

What about simply using the first quota-link? @nickvergessen

@enoch85
Copy link
Member

enoch85 commented Jun 9, 2017

Will this PR also recognize the total amount of storage even if a quota isn't set, or will it still say "unlimited"?

@nickvergessen
Copy link
Member Author

I removed the "of Unlimited" because I think it's useless, the blue bar however will should the percentage of the total available space

@nickvergessen
Copy link
Member Author

nickvergessen commented Jun 12, 2017

So it's like this now:
dateien_-nextcloud-_2017-06-12_10 02 14

Can we move it to the bottom, above »Deleted files«?

I can only move it to the bottom when the box shall be below the deleted files link. Or make it the last item in the list. Is that okay? @jancborchardt

@jancborchardt
Copy link
Member

I'd say let's put it last in the list. If we add it to the sticky part on the bottom, it will obscire the list too much on mobile.

Copy link
Member

@LukasReschke LukasReschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super cool stuff 🚀

@MorrisJobke
Copy link
Member

This is needed to put it to the bottom:

position: fixed !important;
bottom: 88px;
width: inherit !important;
background-color: #fff;
border-right: 1px solid #eee;
z-index:1;

We also need to update the padding of the last element in the upper menu to have a padding of 44px by default:

.app-files #app-navigation > ul li:nth-last-child(2) {
  margin-bottom: 44px;
}

and update the same rule in the trashbin to have 88px (two menu entries):

.app-files #app-navigation > ul li:nth-last-child(2) {
  margin-bottom: 44px;
}

let me update the CSS.

@MorrisJobke
Copy link
Member

I fixed the CSS and it now looks like this:

bildschirmfoto 2017-06-12 um 17 04 24

👍 from me now. (putting the DOM element to the bottom as well messes too much with the rest of the layout so I kept it where it is)

nickvergessen and others added 4 commits June 13, 2017 11:17
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@LukasReschke
Copy link
Member

Rebased on master for test execution.

@LukasReschke LukasReschke added 4. to release Ready to be released and/or waiting for tests to finish 3. to review Waiting for reviews and removed 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish labels Jun 13, 2017
@LukasReschke
Copy link
Member

@MorrisJobke Mind committing your patch for #5305 (comment)?

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the quota bar to not have an icon 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 13, 2017
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@jancborchardt
Copy link
Member

Looked better before with the icon, in line with the others. Right now it might even be confused as being info of the Deleted files entry.

@MariusBluem
Copy link
Member

Looked better before with the icon, in line with the others. Right now it might even be confused as being info of the Deleted files entry.

The quota bar is something different then the sidebar items (deleted files, ...). It is a static element and not something you should/can click on ;)

@jancborchardt
Copy link
Member

It absolutely should be possible to click on in the future so you see how the storage is used. Like on iOS or Android where you see which apps / filetypes use most.

@nickvergessen
Copy link
Member Author

It absolutely should be possible to click on in the future so you see how the storage is used.

Yeah but that is not part of todays story.

Currently 4 people prefer without the icon, and only you prefer with it. Do your thing, or merge. This needs to be merged and backported by friday, please take care of this @jancborchardt if you block this now.

@jancborchardt
Copy link
Member

I did do it like this and it was overwritten. But sure, do design by voting.

@MorrisJobke
Copy link
Member

It absolutely should be possible to click on in the future so you see how the storage is used. Like on iOS or Android where you see which apps / filetypes use most.

But currently on iOS and Android this item also doesn't have an item. So I think this is more in line with that, but as always: we can improve on this if the icon is the better way: sure - we should go for it. Let's see what the users say.

@MorrisJobke MorrisJobke merged commit 666e4de into master Jun 13, 2017
@MorrisJobke MorrisJobke deleted the add-quota-to-files-view branch June 13, 2017 17:19
@MorrisJobke
Copy link
Member

I will take care of the backport.

@skjnldsv
Copy link
Member

Nice work here! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement feature: files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants