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.

Sign up for GitHub

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

Jump to bottom

fix(files): Implement SortingService to fix sorting of files #45419

Merged
merged 2 commits into from
May 27, 2024
Merged

fix(files): Implement SortingService to fix sorting of files #45419

merged 2 commits into from
May 27, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 21, 2024 •
edited
Loading

Summary

The previously used library was parsing strings to try to detect dates, but for filenames it makes no sense to parse them as dates.
Overall the library is parsing a lot of stuff we do not need, so instead of a 3rd party library we can use the native browser solution and use Intl.Collator.

Screenshots

before after
Screenshot 2024-05-21 at 14-54-02 Files - Nextcloud Screenshot 2024-05-21 at 14-53-41 Files - Nextcloud

Checklist

@susnux susnux added this to the Nextcloud 30 milestone May 21, 2024
@susnux susnux requested a review from a team May 21, 2024 12:48
@susnux susnux requested a review from skjnldsv as a code owner May 21, 2024 12:48
@susnux susnux requested review from Fenn-CS and szaimen and removed request for a team May 21, 2024 12:48
@susnux susnux changed the title Fix/files sorting fix(files): Implement SortingService to fix sorting of files May 21, 2024
@susnux susnux requested review from artonge and Pytal May 21, 2024 12:50
@susnux susnux self-assigned this May 21, 2024
@susnux
Copy link
Contributor Author

susnux commented May 21, 2024

/backport to stable29

@susnux
Copy link
Contributor Author

susnux commented May 21, 2024

/backport to stable28

skjnldsv
skjnldsv reviewed May 22, 2024
View reviewed changes
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Having this in the @nextcloud/files would be helpful 🙈

@susnux
Copy link
Contributor Author

susnux commented May 22, 2024

@skjnldsv what of this? The orderBy function? Not really files related more a generic sorting function.
And the whole files sorting is very implementation related, meaning we need to know the current view, the user settings etc.

The only place I know where this could be reused would be the file picker. Or do you have something other in mind?

@skjnldsv
Copy link
Member

Or do you have something other in mind?

Viewer

@susnux
Copy link
Contributor Author

susnux commented May 22, 2024

But do we want to backport the files library update then on stable28 and stable29? If not we need to merge this for stable branches.

@skjnldsv
Copy link
Member

But do we want to backport the files library update then on stable28 and stable29? If not we need to merge this for stable branches.

Fair point!! Let's migrate this after backports to the lib ?

@susnux
Copy link
Contributor Author

susnux commented May 22, 2024

Let's migrate this after backports to the lib ?

Yes sounds good!

@susnux susnux mentioned this pull request May 22, 2024
Merged
szaimen
szaimen approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

@susnux
fix(files): Implement SortingService to fix sorting of files
0e7498b 
The previously used library was parsing strings to try to detect dates,
but for filenames it makes no sense to parse them as dates.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
chore: Compile assets
ed77320 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux enabled auto-merge May 27, 2024 14:16
@susnux susnux merged commit 210117a into master May 27, 2024
104 checks passed
@susnux susnux deleted the fix/files-sorting branch May 27, 2024 14:42
This was referenced May 27, 2024
Merged
Merged
@susnux susnux mentioned this pull request May 27, 2024
Merged
4 tasks
@kesselb kesselb mentioned this pull request Jun 12, 2024
Closed
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@szaimen szaimen szaimen approved these changes

@Fenn-CS Fenn-CS Awaiting requested review from Fenn-CS Fenn-CS was automatically assigned from nextcloud/server-frontend

@artonge artonge Awaiting requested review from artonge

@Pytal Pytal Awaiting requested review from Pytal

Assignees

@susnux susnux

Labels
3. to review Waiting for reviews bug feature: files
Projects
📁 Files team
Archived in project
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.

[Bug]: File sorting based on numbers (with points ) changed in NC 28
3 participants
@susnux @skjnldsv @szaimen