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

skjnldsv · 2018-07-10T10:45:56Z

Somehow broke #9720 (sorry @newhinton) :(

Please review @nextcloud/designers
Already a 👍 from me!

juliusknorr

Toggling the quick access list is not possible
I think we should hide it by default, otherwise some users who already have a bunch of favorites could end up with a long list of folders there and the other sections would be hidden on the first run
Adding a folder to the favorites will not update the list in the sidebar
Clicking the folder flashes a second indicator, which looks a bit weird.
There is also a JS error when clicking a folder in the sidebar:

Uncaught Error: Syntax error, unrecognized expression: li[data-id=http://localhost:8140/index.php/apps/files/?dir=/4 ]
    at Function.ga.error (core.js?v=d502b209-3:2)
    at ga.tokenize (core.js?v=d502b209-3:2)
    at Function.ga [as find] (core.js?v=d502b209-3:2)
    at a.fn.init.find (core.js?v=d502b209-3:2)
    at a.fn.init.a.fn.find (core.js?v=d502b209-3:7)
    at Navigation.setActiveItem (merged-index.js?v=d502b209-3:11396)
    at Navigation._onClickItem (merged-index.js?v=d502b209-3:11421)
    at HTMLDivElement.dispatch (core.js?v=d502b209-3:3)
    at HTMLDivElement.r.handle (core.js?v=d502b209-3:3)

juliusknorr · 2018-07-10T13:34:21Z

@newhinton Besides those comments, I really like it.

@skjnldsv @newhinton Drag-and-drop is not included in the PR, right?

newhinton · 2018-07-11T07:28:02Z

@juliushaertl Which browser do you use? I cannot reproduce the behavior you are describing.

Adding a folder to the favorites will not update the list in the sidebar

Do you mean the ordering is not updated?

There is also a JS error when clicking a folder in the sidebar:

I'll investigate and fix this

Toggling the quick access list is not possible
Clicking the folder flashes a second indicator, which looks a bit weird.

I cant reproduce those

Drag-and-drop is not included in the PR, right?

At the moment it is only disabled. The code is still there, because i thougt that the "how to sort"-discussion wasn't over yet. I have the idea of an dropdown menu, which allows changing of the sorting-order, but i wanted a statement for that because that would introduce a dropdown-menu to the ui, which needs to be discussed. If you want to take a peak, look at newhinton:WithSettingsDropdown. I have tested it there.

skjnldsv · 2018-07-11T07:33:20Z

Toggling the quick access list is not possible
Clicking the folder flashes a second indicator, which looks a bit weird.

I cant reproduce those

I can, I'll fix it!

At the moment it is only disabled. The code is still there, because i thougt that the "how to sort"-discussion wasn't over yet. I have the idea of an dropdown menu, which allows changing of the sorting-order, but i wanted a statement for that because that would introduce a dropdown-menu to the ui, which needs to be discussed. If you want to take a peak, look at newhinton:WithSettingsDropdown. I have tested it there.

@newhinton you can push to this pr!
Nonetheless, let's not add a dropdown, @jancborchardt is not fully on board for now, so let’s just sort them by last modified up top. :)

Is the drag and drop working?
If not, please remove old code and we'll see in another pr if we want this feature or not ;)

juliusknorr · 2018-07-11T07:34:03Z

@newhinton Chrome 67

Adding a folder to the favorites will not update the list in the sidebar
Do you mean the ordering is not updated?

The folder that is added to the favorites is missing in the sidebar until i reload the whole page.

newhinton · 2018-07-11T08:32:06Z

@juliushaertl Sorry, i was confused there, drag&drop is not implemented at all. What i meant was the custom order, which is ordered by drag&drop :D so, nevermind :D

I tested it with firefox and opera, so there may have slipped something

@skjnldsv thanks for fixing it!

Regarding old code, what about the different sorting methods? Currently, a value stored in the server decides about the sorting mechanism. There is currently no way of influcencing this value (because no ui for it, which is fine, don't get me wrong) except from directly calling the api with a proper value. I'd like to keep this, because it is not so much more code, and it allows a user who knows this to change the order. It's somewhat of a hidden feature, which is not optimal in any way, but dumping it is also not optimal (in my opinion). What should i do with it? It's not directly old code, since it does exactly what it is supposed to do

skjnldsv · 2018-07-11T14:34:04Z

apps/files/lib/Controller/ViewController.php

+		foreach ($favElements['folders'] as $elem) {
+
+			$id = substr($elem, strrpos($elem, '/') + 1, strlen($elem));
+			$link = $this->urlGenerator->linkToRouteAbsolute('files.view.index', ['dir' => $elem]);


Somehow the link generated here does not redirect to the proper view in fies :/
@juliushaertl any clue?

clicking this redirect from /apps/files/?dir=/Test
to /apps/files/dir=/&view=Test

Fixed now

MorrisJobke

Tested and works 👍

MorrisJobke · 2018-07-12T11:21:41Z

Unit test failure:

1) OCA\Files\Tests\Controller\ViewControllerTest::testIndexWithRegularBrowser
Your test case is not allowed to access the database.

skjnldsv · 2018-07-12T11:23:14Z

Unit test failure:

Any clue? I'm on the jsunit atm :)

MorrisJobke · 2018-07-12T11:42:21Z

apps/files/lib/Controller/ViewController.php

+		$helper = new \OCA\Files\Activity\Helper($tagger);
+
+		try {
+			$favElements = $helper->getFavoriteFilePaths($this->userSession->getUser()->getUID());


This one is likely to cause the "DB used" exception. This one is called in a test that has no DB but actually needs one.

I think I fixed the tests

juliusknorr

Works great for me now 👍

Good to go, if the jsunit tests are fixed. 😉

Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed duplicate collapse-button and changed api-endpoints Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed app-navigation-caption from apps.scss Signed-off-by: fnuesse <felix.nuesse@t-online.de> Changed api-endpoints Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Codestyle (.js) Signed-off-by: fnuesse <felix.nuesse@t-online.de> Hid away extended Settings Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed reverse state Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Missing reverse after changing sort-strategy Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Copyright-Header Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed UI-Flickering Signed-off-by: fnuesse <felix.nuesse@t-online.de> hid dotmenu on toggle while favorites are empty Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added Draggable to listelements (WIP) Signed-off-by: fnuesse <felix.nuesse@t-online.de> Rebuild appnavigation.php with recursive function to allow easy implementation of sublists Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed draggable Sublist-Elements Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed draggable Sublist-Elements Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added date-modified sorting option to quickaccess Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added custom order sorting option to quickaccess Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added custom order sorting option to quickaccess Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added fallback for custom ordering Signed-off-by: fnuesse <felix.nuesse@t-online.de>

Signed-off-by: fnuesse <felix.nuesse@t-online.de>

Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Bad url-generation in javascript for new quickaccessitems Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed vertical scrolling in sortable-list which leads to "hidden" navbar Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed unnessessary console logs Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Bounds in custom sorting Signed-off-by: fnuesse <felix.nuesse@t-online.de> Reformatted code Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed horizontalscroll on sortable-list Fixed "stuck element" where you could not switch back to the original ordering in the sortable-list Signed-off-by: fnuesse <felix.nuesse@t-online.de>

Signed-off-by: fnuesse <felix.nuesse@t-online.de> Added icon-change on drag Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed Navbar-closing in app when favorites-list is toggled on mobile Signed-off-by: fnuesse <felix.nuesse@t-online.de> Refactored Code Signed-off-by: fnuesse <felix.nuesse@t-online.de> Changed to alphabetical sorting Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed deletion of folder with identical names Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed ability to add files to the quickaccess Signed-off-by: fnuesse <felix.nuesse@t-online.de> Fixed wrong path-generation when added from favorites-star Signed-off-by: fnuesse <felix.nuesse@t-online.de> Removed Element from navbar when favorite-star in detailview is toggled off Signed-off-by: fnuesse <felix.nuesse@t-online.de> Changed Quota-Text to prevent boundarybreaks Reverted last commit

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

skjnldsv · 2018-07-12T15:51:34Z

All fixed, let's wait for final tests
https://drone.nextcloud.com/nextcloud/server/8883

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer · 2018-07-12T18:23:11Z

phpunit test broken! fix pushed

newhinton · 2018-07-12T21:49:01Z

@skjnldsv I see you have removed the _setOnDrag function, i get that, but why do you keep all the other stuff, which is now dead code?

If you had kept it, the feature would still work, even if you couldnt toggle it in the ui, and if you wanted to clean up code, there is a lot missing

skjnldsv · 2018-07-13T05:35:03Z

@newhinton I removed it because the drag wasn't working properly :)
I thought I cleaned all the related stuff, could you point out where there are still stuff left please?

Thanks for your amazing work, this pr is a very nice addition to nc14!!

newhinton · 2018-07-13T09:22:20Z

@skjnldsv I would prefer to fix it instead of removing it, what do you say?

skjnldsv · 2018-07-13T09:24:30Z

Well, depend on what you wish to fix, but in the end we decided to remove the custom sorting, so if there are still stuff laying around, please remove them.
Though drag and drop was a neat idea, so if you want to add them back, please do :)

ps: I fixed some errors in another issue, currently the addition/removal is broken, I forgot to edit something, do don't worry about this, I'll handle it :)

newhinton · 2018-07-13T09:36:55Z

In that case i meant the sortable handler (for custom sorting). While we are not using it for this feature, the handling of different sorting strategies is fully working in the current files-app, and the only thing currently breaking sortablelist is the missing handler.
When we would readd the handler, it would work again (after fixing your mentioned bugs).

This enables us to use this feature later, when (and if) we found a solution for the ui-problem (look at newhinton:WithSettingsDropdown , i have experimented there) without redeveloping everything.
Also, slight changes could make the draggablething dynamic for multiple lists, like the upcoming pr for the shared entries, as an example.

If you want to remove it, (i can remove it, if you whish) you need to delete the api-routes, you need to remove the unnessessary sorting-variable in navigation.js, edit the quicksort-helper function to only return the alphabetical sorting value and edit the php-code to dump the initial-sorting.

I believe my solution for sorting is quiet dynamic and expandable there, and while not perfect, provides a lot of possibilities and i would keep it, but in the end, it's your decision ;)

skjnldsv · 2018-07-13T09:42:57Z

No, we don't want to add a menu of any sort in the end. Let keep things simple, the ui already start to be a bit overcrowded :)

Please, go ahead for the cleanup, I'm on another issue ;)
Thanks a lot!

newhinton · 2018-07-13T14:54:56Z

@skjnldsv How do i push the cleanup? Should i open a new PR?

skjnldsv · 2018-07-13T14:57:32Z

Yes, on nextcloud/server please :)

skjnldsv added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews feature: files labels Jul 10, 2018

skjnldsv added this to the Nextcloud 14 milestone Jul 10, 2018

skjnldsv self-assigned this Jul 10, 2018

skjnldsv requested review from rullzer, MorrisJobke, jancborchardt and juliusknorr July 10, 2018 10:45

skjnldsv mentioned this pull request Jul 10, 2018

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

Closed

juliusknorr previously requested changes Jul 10, 2018

View reviewed changes

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 11, 2018

skjnldsv commented Jul 11, 2018

View reviewed changes

skjnldsv requested review from juliusknorr and danxuliu July 12, 2018 07:59

MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 12, 2018

MorrisJobke approved these changes Jul 12, 2018

View reviewed changes

MorrisJobke reviewed Jul 12, 2018

View reviewed changes

juliusknorr approved these changes Jul 12, 2018

View reviewed changes

newhinton and others added 9 commits July 12, 2018 16:49

Set Quickaccess-ordering to custom order

9b914bd

Signed-off-by: fnuesse <felix.nuesse@t-online.de>

Fix app nav menu highlight

d8c69df

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Default state to collapsed and fixed code styling

5470b95

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Fixed invalid uid

5bac969

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Fixed favorite folder view display

ae86845

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Fixed tests

bd1929a

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

skjnldsv force-pushed the newhinton/master branch from 82de4d1 to bd1929a Compare July 12, 2018 14:49

rullzer added 2 commits July 12, 2018 17:49

Inject helper

e56a907

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Fix tests

999dc64

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 12, 2018

No draggable list anymore

49a701e

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

skjnldsv merged commit 052221a into master Jul 12, 2018

skjnldsv deleted the newhinton/master branch July 12, 2018 20:54

This was referenced Jul 15, 2018

Add Quick-Access to favorite folder in left sidepanel in files-app (Cleanup) #10246

Closed

Cleanup of leftover-sorting-code from quickaccess-feature #9714 #9720 #10320

Merged

skjnldsv mentioned this pull request Jan 10, 2020

Fix webdav url #18804

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

skjnldsv commented Jul 10, 2018

juliusknorr left a comment •

edited by skjnldsv

Loading

juliusknorr commented Jul 10, 2018

newhinton commented Jul 11, 2018

skjnldsv commented Jul 11, 2018 •

edited

Loading

juliusknorr commented Jul 11, 2018

newhinton commented Jul 11, 2018 •

edited

Loading

skjnldsv Jul 11, 2018

skjnldsv Jul 11, 2018

MorrisJobke left a comment

MorrisJobke commented Jul 12, 2018

skjnldsv commented Jul 12, 2018

MorrisJobke Jul 12, 2018 •

edited

Loading

rullzer Jul 12, 2018

juliusknorr left a comment

skjnldsv commented Jul 12, 2018

rullzer commented Jul 12, 2018

newhinton commented Jul 12, 2018 •

edited

Loading

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018 •

edited

Loading

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018

skjnldsv commented Jul 13, 2018

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

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

Conversation

skjnldsv commented Jul 10, 2018

juliusknorr left a comment • edited by skjnldsv Loading

Choose a reason for hiding this comment

juliusknorr commented Jul 10, 2018

newhinton commented Jul 11, 2018

skjnldsv commented Jul 11, 2018 • edited Loading

juliusknorr commented Jul 11, 2018

newhinton commented Jul 11, 2018 • edited Loading

skjnldsv Jul 11, 2018

Choose a reason for hiding this comment

skjnldsv Jul 11, 2018

Choose a reason for hiding this comment

MorrisJobke left a comment

Choose a reason for hiding this comment

MorrisJobke commented Jul 12, 2018

skjnldsv commented Jul 12, 2018

MorrisJobke Jul 12, 2018 • edited Loading

Choose a reason for hiding this comment

rullzer Jul 12, 2018

Choose a reason for hiding this comment

juliusknorr left a comment

Choose a reason for hiding this comment

skjnldsv commented Jul 12, 2018

rullzer commented Jul 12, 2018

newhinton commented Jul 12, 2018 • edited Loading

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018 • edited Loading

skjnldsv commented Jul 13, 2018

newhinton commented Jul 13, 2018

skjnldsv commented Jul 13, 2018

juliusknorr left a comment •

edited by skjnldsv

Loading

skjnldsv commented Jul 11, 2018 •

edited

Loading

newhinton commented Jul 11, 2018 •

edited

Loading

MorrisJobke Jul 12, 2018 •

edited

Loading

newhinton commented Jul 12, 2018 •

edited

Loading

newhinton commented Jul 13, 2018 •

edited

Loading