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

WTrackMenu: sort crates and playlists like in sidebar #11023

Merged
merged 2 commits into from
Oct 31, 2022
Merged

WTrackMenu: sort crates and playlists like in sidebar #11023

merged 2 commits into from
Oct 31, 2022

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 29, 2022 •
edited
Loading

In the sidebar crates and playlists are sorted lexicographically.
This PR uses the same order for the crates and playlists submenus.
image

@ronso0 ronso0 linked an issue Oct 29, 2022 that may be closed by this pull request
Crates sorted differently in Tree and context menu #9546
Closed
@ronso0 ronso0 added this to the 2.3.4 milestone Oct 29, 2022
@Russe
Copy link
Contributor

Russe commented Oct 30, 2022

I can confirm this will fix the sort issue with crates and playlists, tested on Ubuntu.

@daschuer daschuer added the changelog This PR should be included in the changelog label Oct 31, 2022
daschuer
daschuer requested changes Oct 31, 2022
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

It works good just some C++ improvements suggested.

src/library/dao/playlistdao.cpp Outdated Show resolved Hide resolved
src/library/dao/playlistdao.cpp Outdated Show resolved Hide resolved
src/widget/wtrackmenu.cpp Outdated Show resolved Hide resolved
src/widget/wtrackmenu.cpp Outdated Show resolved Hide resolved
@daschuer daschuer mentioned this pull request Oct 31, 2022
Open
@ronso0 ronso0 force-pushed the crates-playlists-sorting-tree-vs-menu branch from 7343187 to c16d651 Compare October 31, 2022 19:10
@ronso0
Copy link
Member Author

ronso0 commented Oct 31, 2022 •
edited
Loading

I squashed your proposals, should be fine now.

Edit nope, error with the id in lambda and for (const auto& [id, name] : playlists)

@ronso0 ronso0 force-pushed the crates-playlists-sorting-tree-vs-menu branch from c16d651 to ab0c8b9 Compare October 31, 2022 20:31
daschuer
daschuer approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM and still works. Thank you.

@daschuer daschuer merged commit af621c0 into mixxxdj:2.3 Oct 31, 2022
@ronso0 ronso0 deleted the crates-playlists-sorting-tree-vs-menu branch October 31, 2022 23:01
@ronso0 ronso0 mentioned this pull request Jan 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daschuer daschuer daschuer approved these changes

Assignees
No one assigned
Labels
changelog This PR should be included in the changelog library ui
Projects
None yet
Milestone
2.3.4
Development

Successfully merging this pull request may close these issues.

Crates sorted differently in Tree and context menu
3 participants
@ronso0 @Russe @daschuer