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

Folders sorting incorrectly across pagination #893

Closed
5 of 6 tasks
scott1702 opened this issue Dec 12, 2018 · 11 comments
Closed
5 of 6 tasks

Folders sorting incorrectly across pagination #893

scott1702 opened this issue Dec 12, 2018 · 11 comments

Comments

@scott1702
Copy link
Contributor

scott1702 commented Dec 12, 2018

Overview

The fix here #844 caused another issue where folders will only appear at the top of the page they're on, not across all pages. E.g. if I have 50 pages of files/folders and I have a folder called 'Uploads', this folder may not appear until page ~45. This has a fairly big impact on the UX of asset-admin as content authors will struggle to find their folders.

Acceptance Criteria

The below is documenting what already should be there before the regression was introduced.

  • Shows all folders first in default sort
  • Paginates across folders and files
  • Shows folders visually separate from files if both types are on the same page
  • Works with fluent
  • Doesn't show folders first when changing sort order to "recently edited")

PRs

@robbieaverill
Copy link
Contributor

So I understand that folders used to be shown first, before files, but now that has regressed and is no longer the case. This means that if you have one folder (Uploads) and 45 pages of files, it won't show up until page 45.

If that's the case, I'll triage as a high impact bug/regression

@chillu
Copy link
Member

chillu commented Apr 7, 2019

There's two ways we could do this. Examples with 50 folders and 10 files

  1. Fix the regression
page 1: folder 1-20
page 2: folder 21-40
page 3: folder 41-50, files 1-10
  1. Paul's alternative suggestion for new UI:
page 1: folder 1-10, files 1-10
page 1*: folder 1-50, files 1-10

We'd need either loading of all folders (cap at 100?), or we need infinite scrolling.

@tomaszpirc
Copy link

tomaszpirc commented Apr 12, 2019

Both solutions have serious drawbacks when handling large assets libraries. Maybe the most intuitive solution would be using for folders the same Treeview that is used for Pages. This would:

  • bring more coherence into both sections;
  • allow accessing and listing folders separately from files (avoid both awkward situations from above proposals);
  • offer users a familiar interface (used by Wins and OS)

What do you think?

@dnsl48
Copy link
Contributor

dnsl48 commented Apr 14, 2019

In my opinion any option will heavily depend on a project workflow.
I believe we have to revert the regression and bring back the original (4.0) behaviour first, then we could take a step back, rethink this, then gradually introduce new functionality or postpone the change until SS5.
Eventually, we should even be able to switch UI behaviour as long as API provides both options:

  • get a list of mixed-up files + folders
  • get files and folders separately with 2 different API calls

@dnsl48
Copy link
Contributor

dnsl48 commented Apr 14, 2019

/cc @silverstripeux

@clarkepaul
Copy link
Contributor

clarkepaul commented Apr 15, 2019

Just an FYI, this icon was supposed to present a tree view of the folder structure using the multiselect dropdown field. Currently it just links directly to edit the folder which was a temporary thing which never progressed (the edit link was to be at the top of the dropdown – can't find designs).
Pasted_Image_16_04_19__9_49_AM

See https://projects.invisionapp.com/dsm/silver-stripe/silver-stripe/folder/components/5cb5141972757b2f9d8e37d7

@ScopeyNZ
Copy link
Contributor

I'm pretty sure this was in our pattern-lib at some point but I can't find it now. I don't think it either took you to edit or presented a tree view - I remember it allowing you to edit the folder name in-line.

Hopefully I didn't imagine all of that.

@clarkepaul
Copy link
Contributor

Currently in the CMS it takes you to the edit panel, not sure about it being inline editable though?

dnsl48 added a commit to open-sausages/silverstripe-asset-admin that referenced this issue Apr 17, 2019
dnsl48 added a commit to open-sausages/silverstripe-asset-admin that referenced this issue Apr 17, 2019
@dnsl48
Copy link
Contributor

dnsl48 commented Apr 17, 2019

Shows all folders first in default sort (by name asc or desc)

I checked the 4.1 behaviour and that's a bit different.
Basically, "Folders first" only kicks in when there's no manual ordering picked.
If user picks asc or desc for title, then "folders first" doesn't happen anymore.
That doesn't seem to be intentional though, but rather caused by silverstripe/silverstripe-framework#8926 (that's my assumption at the moment, I didn't dig too deeply into it)

I'm still going to reproduce that original behaviour in sake of backward compatibility.

@chillu
Copy link
Member

chillu commented Apr 17, 2019

Yep, I've removed the "(by name asc or desc)" from the AC, good catch

@dnsl48
Copy link
Contributor

dnsl48 commented Apr 18, 2019

  • Shows folders visually separate from files if both types are on the same page
  • Doesn't show folders first when changing sort order to "recently edited")

Currently the former AC takes precedence and when files with folders are on the same page, folders go first even with custom ordering.
I'm leaving that behaviour as is, though it may appear a little bit confusing.

@dnsl48 dnsl48 removed their assignment Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants