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 Quick-Access to favorite folder in left sidepanel in files-app #9714

Closed
newhinton opened this issue Jun 1, 2018 · 39 comments
Closed

Add Quick-Access to favorite folder in left sidepanel in files-app #9714

newhinton opened this issue Jun 1, 2018 · 39 comments

Comments

@newhinton
Copy link
Contributor

newhinton commented Jun 1, 2018

It would be a good feature to add a quick way to access favorite folders on the "main" view of the file app. Most Fileexplorer have such a feature to access often used directories quickly.

I have already started implementing this in the files-app, i am creating this Issue mostly to gather feedback and to check if the files-app is the right place to implement this (in opposition to an additional app wich extends the original app ( I dont know how to do this)).

This feature is going to be toggleable.
u14qxz0h

@c0fe
Copy link

c0fe commented Jun 1, 2018

This looks like a pretty useful feature to be honest.

@MorrisJobke @rullzer any thoughts?

@MorrisJobke
Copy link
Member

Cc @nextcloud/designers

@enoch85
Copy link
Member

enoch85 commented Jun 2, 2018

👍

@newhinton
Copy link
Contributor Author

I have added a pull request.

The only thing which does not properly work, is the initial State of the checkbox which enables this functionality. It is properly set, but the initial state is always unchecked, regardless of the true value.

@newhinton
Copy link
Contributor Author

newhinton commented Jun 3, 2018

The trashbin and quota-indicator are list-elements which are pushed down to "dock" on the settings button. If the folderlist exceeds the size of the sidebar, it gets a scrollbar, which is fine.
The problem: Quota and Trashbin are listelements and therefore they are pushed "under" the settingsbutton. Should they stay over that button, or is it acceptable to let them slide down?

vyljoecz

@skjnldsv
Copy link
Member

skjnldsv commented Jun 3, 2018

@newhinton no, it is the expected behaviour :)
This is intended for better accessibility on small height views which reduced the total height and show too few of the first navigations items.

@MorrisJobke MorrisJobke added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 4, 2018
@MorrisJobke MorrisJobke changed the title [Feature] Add Quick-Access to favorite folder in left sidepanel in files-app Add Quick-Access to favorite folder in left sidepanel in files-app Jun 4, 2018
@juliushaertl
Copy link
Member

While I like the idea of having favorite folders directly accessible, I fear that this will clutter the UI too much once you have more folders marked as favorite. Maybe we could just limit it to first level favorite folders or have a hard limit of 3-4 entries? Anyhow having a limit would only make sense if we could show the most recently used folders or something similar.

cc @jancborchardt

@skjnldsv
Copy link
Member

skjnldsv commented Jun 4, 2018

We also supports collapsible entries, we could go for this?

@newhinton
Copy link
Contributor Author

newhinton commented Jun 4, 2018

A stylechange could help differentiate between menu-entries and favorites. Making favorites smaller could clean up the UI, but I'll need to try that out and see if it works

@c0fe
Copy link

c0fe commented Jun 4, 2018

@juliushaertl what about making this into an accordion style menu where if you click you will get the full list? or perhaps show a number of items then a button "show more"?

@newhinton
Copy link
Contributor Author

newhinton commented Jun 4, 2018

Also, this is not enabled by default, so the user needs to actively decide to "clutter" his UI, so it would be an active decision by the user

@skjnldsv
Copy link
Member

skjnldsv commented Jun 4, 2018

Also, this is not enabled in default, so the user needs to actively decide to "clutter" his UI, so it would be an active decision by the user

Yeah, in the end, maybe it's not that big of a deal! :)

@newhinton
Copy link
Contributor Author

@juliushaertl
After rethinking the problem, another way to solve this, would be another attribute, like quickaccess or something. We could set it in the folder contextmenu, and only show those items

The upsides:
reduces cluttering for existing users with many favorites
seperates those two features

The downsides:
adds complexity
has the ability to make favorites obsolete, because often you dont need both
wouldn't really solve the sidebar cluttering

@jancborchardt
Copy link
Member

jancborchardt commented Jun 7, 2018

Some design feedback:

  • There should be no setting for this, it's good to be enabled
  • As @skjnldsv says it should be collapsible. Last state should be remembered
  • It should be subentries of the "Favorites" navigation entry instead of a separate list
  • No additional attribute for this, as this is unnecessarily complex and confusing.
  • The list should be limited to probably 5 entries, with "Show all favorites" below to expand. Check the Mail app how we do it for Mail folders cc @ChristophWurst

@skjnldsv
Copy link
Member

skjnldsv commented Jun 7, 2018

No additional attribute for this, as this is unnecessarily complex and confusing.

What do you mean @jancborchardt ? :)

There should be no setting for this, it's good to be enabled

I agree, let simplify this! :)

@jancborchardt
Copy link
Member

@skjnldsv what @newhinton said above:

After rethinking the problem, another way to solve this, would be another attribute, like quickaccess or something. We could set it in the folder contextmenu, and only show those items

We shouldn't do that – not another separate attribute.

@c0fe
Copy link

c0fe commented Jun 7, 2018

@skjnldsv is it possible to make that scrollbar much thinner? That current state, it is pretty ugly.

@skjnldsv
Copy link
Member

skjnldsv commented Jun 7, 2018

@c0fe it depends on browsers mostly :)
But yes, we can try to override that. iirc we already have some fixes for that!

@c0fe
Copy link

c0fe commented Jun 7, 2018

I am thinking of the modern ones, Firefox Quantum and Chrome along with Microsoft Edge and Internet Explorer 11. I am thinking of replacing the gray thick scrollbar with a light blue and thinner one, one that would match Nextcloud color scheme more.

@skjnldsv
Copy link
Member

skjnldsv commented Jun 7, 2018

@c0fe please suggest pr and screenshots :D

@newhinton
Copy link
Contributor Author

@skjnldsv
If you tell me where an example is, i'll try to include it in the next changes

@skjnldsv
Copy link
Member

skjnldsv commented Jun 7, 2018

@newhinton about the scrollbar?
No, it's better to split pull requests :)

@newhinton
Copy link
Contributor Author

Okay :)

@c0fe
Copy link

c0fe commented Jun 7, 2018

@skjnldsv ok will do

@newhinton i will just create an PR once I see your PR

@newhinton
Copy link
Contributor Author

newhinton commented Jun 7, 2018

i have updated the style and disabled the ability to turn this feature off.

This is the current style:

c6anzs4u

@jancborchardt I am really not quiet fond of that whole collapsible idea. It stands in conflict to my "one-click-to-destination"-approach most common desktop-fileexplorers use.
There is already a way to access the whole list with an additional click, so i would try to reduce it to the bare minimum.
Also, since there is no preselection of the first n elements of that collapsed list, it wouldn't really help the user, since the only sensible way of sorting is to sort by alphabet or most recent, which are not quiet right for this feature.
And it will take considerable time to change the code to be able to collapse in this navbar.

@newhinton
Copy link
Contributor Author

newhinton commented Jun 7, 2018

I have experimented with this idea, and while the Content of those links are placeholder, i think the rest looks quiet good

2018-06-07 17_55_04-window

I think there is a little triangle missing :D

@skjnldsv
Copy link
Member

skjnldsv commented Jun 7, 2018

@newhinton Did you edit the style of the subitems?
It is not supposed to look like that at all! 🤔

EDIT: see review: #9720 (review)

@newhinton
Copy link
Contributor Author

newhinton commented Jun 7, 2018

@skjnldsv I added some new ones to experiment with different styles.
None of the styles before solved the cluttering problem, or even made this problem smaller.
While the latest approach does not solve it fully, and introduces some organizational problems, it takes a clean and simple approach, which has a really small footprint.
It also creates a clear distinction between two different elements of the navbar, the "main"-elements, like Favorites, Recent or All Files, and a second type of element, which has a completely different function, in this case directly hardlinking to a specific folder.
I think those elements should be clearly visually seperated, which i achieved with this.
And in the end, this is still a WIP, so we should be able to find a solution for this

@skjnldsv
Copy link
Member

skjnldsv commented Jun 8, 2018

Aside from the new caption items, you should not use anything else than what's in the docs, or we'll lose all the unification work we did :)

@newhinton
Copy link
Contributor Author

Those are my two versions. I dont think that the big one is really what we want, it just does not look "clean", in my opinion it's bulky and unnessessary obtrusive

2018-06-13 09_06_22-

@skjnldsv
Copy link
Member

@newhinton See #9720 (review)

.app-navigation-subelement
Okay, I get it now! :)
There is already a standard for this:
Look at our documentation : https://docs.nextcloud.com/server/13/developer_manual/design/navigation.html

@skjnldsv skjnldsv added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 13, 2018
@newhinton
Copy link
Contributor Author

@skjnldsv
Yes, i have seen it, but that's not the point i wanted to make. I dont think that the Quickaccess is a feature we should introduce, because it would create more ui-clutter and would be highly unorganized with the current stlyeguide. Which is not what we want, and as a result of that, we should not include it until a good solution can be found.

@newhinton
Copy link
Contributor Author

newhinton commented Jun 17, 2018

2018-06-17 16_11_32-window

This uses, more or less, stock css

@newhinton
Copy link
Contributor Author

@skjnldsv I have changed the design, with the styleguide in mind. I have also added sorting to this list, this should make custom ordering easier to implement in the future.

@skjnldsv
Copy link
Member

This is looking really great!!!!

@newhinton
Copy link
Contributor Author

newhinton commented Jul 20, 2018

@skjnldsv Can i close this issue, and if yes, how do you handle successful issue-closing? Is there a tag to mark it as successfully closed other than the label '4. to release' ? Or should we do it after the merge of #10320?

@newhinton
Copy link
Contributor Author

newhinton commented Jul 20, 2018

@skjnldsv Oh, and i have seen that the star-icon was removed for the Navbarelement, what was the design desicion behind that? To me it looks a little bit strange that there is only blank space, it seems so empty :) (Same for Shares)

@skjnldsv
Copy link
Member

@newhinton, no descision, just an error, we'll add it back :)

rullzer pushed a commit that referenced this issue Jul 24, 2018
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
skjnldsv pushed a commit that referenced this issue Aug 8, 2018
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
skjnldsv added a commit that referenced this issue Aug 8, 2018
…leanup

Cleanup of leftover-sorting-code from quickaccess-feature #9714 #9720
weeman1337 pushed a commit that referenced this issue Aug 9, 2018
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants