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

Assets tree is lexicographic sorted instead of natural sorted #4370

Closed
tugboat-group opened this issue May 14, 2019 · 8 comments

Comments

Projects
None yet
4 participants
@tugboat-group
Copy link

commented May 14, 2019

Bug Report

Expected behavior

Assets tree files should be sorted by a natural sort.
i.e:
test1
test2
test3
test12
test13

On my Mac
Mac-natural-sorting

MySQL 'Order By' - sorting alphanumeric correctly:
https://stackoverflow.com/questions/8557172/mysql-order-by-sorting-alphanumeric-correctly

Actual behaviour

Assets tree files are lexicographic sorted

test1
test10
test11
test12
test2
test3
test4

From the demo instance
Lexi-sort

@tugboat-group tugboat-group changed the title Assets tree is lexicographic sorting instead of natural sorting Assets tree is lexicographic sorted instead of natural sorted May 15, 2019

@brusch brusch self-assigned this May 17, 2019

@brusch brusch added the Improvement label May 17, 2019

@brusch brusch added this to To do in Pimcore 6.0.0 ("Ale") via automation May 17, 2019

@brusch brusch added this to the 6.0.0 milestone May 17, 2019

@brusch brusch assigned weisswurstkanone and unassigned brusch May 20, 2019

@weisswurstkanone

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

Works as designed. Mysql is only one part of the story.

Note that the "show in tree" feature is currently implemented entirely on the client-side as javascript code.

Pimcore 6.0.0 ("Ale") automation moved this from To do to Done May 20, 2019

@brusch brusch removed this from the 6.0.0 ("Ale") milestone May 20, 2019

@brusch brusch removed this from Done in Pimcore 6.0.0 ("Ale") May 20, 2019

@NiklasBr

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

Works as designed.

Maybe the design is not as good as it could be?

@brusch

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Maybe ..., but it's not our top priority.
If you feel this is an essential feature, please feel free to create a PR for us.

@NiklasBr

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

Ok. Can we re-open this issue?

@brusch

This comment has been minimized.

Copy link
Member

commented May 20, 2019

A PR is equivalent to an issue, so just open a PR once you have implemented this feature :)

@tugboat-group

This comment has been minimized.

Copy link
Author

commented May 21, 2019

Works as designed. Mysql is only one part of the story.

Hi Josef,
Could you please add some more information so I'll be able to have a better understanding of the task ahead?

Thanks.

@weisswurstkanone

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

Just to summarize

  • this needs to be done for objects as well (changed the title)
    -the tree data is generated by AssetController::treeGetChildsByIdAction (and DataObjectController). This is the server-side place where the sorting happens (mysql).

"Show in tree" is completely handled on the client-side.

image

Best place to look at:
https://github.com/pimcore/pimcore/blob/095abd441a7507525b6594e1b3da8bb4387d1ff1/bundles/AdminBundle/Resources/public/js/pimcore/treenodelocator.js

It basically decides whether to switch forward (increase the list offset) or backward depending one the asset filename / object key (note that it can also be sorted by indices - so be careful) based on a string comparison.

@tugboat-group

This comment has been minimized.

Copy link
Author

commented May 22, 2019

Thank you for the information Josef.
I now have a better understanding on this update scope and possible implications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.