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

[5.x] Allow asset container order to be specified #10177

Open
wants to merge 8 commits into
base: 5.x
Choose a base branch
from

Conversation

ryanmitchell
Copy link
Contributor

@ryanmitchell ryanmitchell commented May 23, 2024

Since #4947 asset containers have been ordered by title, which is mostly a good thing.

However sometimes, you have an asset container that is low priority and rarely used, but then appears first, which isnt so good. A small quality of life improvement is to let the order by specified, while still falling back to title when its not.

This PR introduces an order key to the yaml, which the CP then sorts by when outputting the list, eg order: 99

Before:

CleanShot 2024-05-23 at 13 59 23@2x

After:

CleanShot 2024-05-23 at 13 58 55@2x

@robdekort
Copy link
Contributor

This works like a charm. Love it.

Copy link
Member

@duncanmcclean duncanmcclean left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I've made a couple of tweaks:

  • Moved the fallback order out of the $order property into the order() method, so the property contains the "file" value.
  • When the $order property is set, it'll now include it in the fileData array so it'll get saved to the asset container's YAML file.
  • Removed the default value from being set in the AssetContainersStore so the $order property is null unless it's been explicitly set in the asset container file, to prevent order: 1 being saved to every asset container.

@jasonvarga
Copy link
Member

Feels like a slippery slope. If asset containers can have custom orders, why not collections, taxonomies, globals, navs, forms, etc.

Does using the nav builder to reorder your containers not work for you? You can already move the more important asset container to the top. Collections, taxonomies, etc all of them too.

I assume once this is merged you will be doing an "Add order to everything else" PR? 😅

Also if you're going to apply this order to the nav and asset browser tabs, you may as well move it up a level and make the sort happen on AssetContainer::all().

Finally, there's no way to apply the order other than adding it to the yaml. You probably will want a field in the CP settings.

Just brain dumping here and looking for feedback, don't just do it. 😃

@ryanmitchell
Copy link
Contributor Author

I think the issue with the nav builder in this case is yes you can change the sidebar order, but it doesn't change the tab order in assets, which this allows. I have no plans for another PR (honest!). Happy for you to close if you dont feel its of benefit... I mostly did this cause @robdekort wanted it.

@robdekort
Copy link
Contributor

robdekort commented May 28, 2024

Yeah, the issue is that the assets directly open up. In the case of my sites you always go to the Favicons container because it happens to be the first in the alphabet. This like only makes sense once during development:
Screenshot 2024-05-28 at 19 04 29

With collection you open up in the overview table so there's no issue there. Fwiw, I have no other order requests, lol :-)

@robdekort
Copy link
Contributor

Just a thought. Perhaps it would make more sense if the CP nav order would be respected by the tabs?

@edalzell
Copy link
Contributor

edalzell commented May 28, 2024

Ya we'd love this too, cuz we have a Private asset container by default in our site builder, but it's rarely used, so we'd like it at the end of the tabs.

We have no issues with any of the other data because they don't really have "tabs" the same way Asset Containers do.

@ebeauchamps
Copy link
Contributor

I am managing a site with tons of assets. I am uploading multiple assets, and only then I am calling some of them when I am creating my content. I need a way to choose the column on which the asset container will be sorted (most often, it's the Date column, descending order). I was indeed surprise that collections only can be sorted with a default order. Will this PR allow the user to choose the column and the sort order, by default? It's critical in my use case. Thanks.

@duncanmcclean
Copy link
Member

I am managing a site with tons of assets. I am uploading multiple assets, and only then I am calling some of them when I am creating my content. I need a way to choose the column on which the asset container will be sorted (most often, it's the Date column, descending order). I was indeed surprise that collections only can be sorted with a default order. Will this PR allow the user to choose the column and the sort order, by default? It's critical in my use case. Thanks.

No, this pull request only allows you to re-order the asset container tabs shown in this screenshot.

CleanShot 2024-05-30 at 15 41 19

@ebeauchamps
Copy link
Contributor

Thanks Jason. I'll take a look into "Ideas" then.

@ryanmitchell
Copy link
Contributor Author

@jasonvarga I've moved the sort to :all() and added a CP field

@morhi
Copy link
Contributor

morhi commented Jun 5, 2024

As a workaround for this I just prefixed the container title with 1 - and 2 - :)

@robdekort
Copy link
Contributor

As a workaround for this I just prefixed the container title with 1 - and 2 - :)

Been there, but it just killed me whenever I saw those digits in the CP 🤣

@jasonvarga
Copy link
Member

Set your container titles to title: '<span class="hidden">1</span>Name'

jk definitely don't do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants