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

Sort albums by PremiereDate on artist screen #901

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

Maxr1998
Copy link
Collaborator

@Maxr1998 Maxr1998 commented Sep 26, 2024

This ensures albums are correctly ordered by release date, if there are more than one within the same year.
I am not sure if the ProductionYear sort is still needed here, but it doesn't hurt, I suppose. If the PremiereDate is empty, that would mean it's at least sorted by ProductionYear like before.

@Maxr1998
Copy link
Collaborator Author

Maxr1998 commented Sep 26, 2024

For what it's worth, the offline sort algorithm already only uses thePremiereDate. Maybe that should be changed too for consistency, or we drop the ProductionYear as mentioned.

@Chaphasilor
Copy link
Collaborator

Thanks, but I'm having a deja-vu 😅
#448

Or did that never end up in redesign?
Also, there's a related issue: #536
Is that getting addressed here? 🤔

@Maxr1998
Copy link
Collaborator Author

Good memory! The previous commit did end up in redesign, but as the artist screen got a separate interface, my change was essentially dropped (as music_screen_tab_view doesn't handle artists anymore).

The linked issue is a bit of a library issue. If the albums don't have years applied for whatever reason, sorting by either ProductionYear or PremiereDate will obviously fail. I agree that adding the SortName a sort parameter would fix this.
My comment in my previous PR reminded me of something, though. It appears that using ProductionYear,PremiereDate as a sorting is pretty useless, since PremiereDate already falls back to ProductionYear in the server source.

PremiereDate already automatically falls back to the ProductionYear on the server-side if missing, so there's no need to specify it explicitly.
If albums don't have a PremiereDate set, they should be sorted by name to have at least some type of order.
@Chaphasilor
Copy link
Collaborator

Okay, that seems good! Right, at some point artist were just a list of albums...

The changes look good to me. I'll go ahead and merge them :)

@Chaphasilor Chaphasilor merged commit ee78769 into jmshrv:redesign Sep 30, 2024
4 checks passed
@Maxr1998 Maxr1998 deleted the sort-artist-albums branch October 1, 2024 10:42
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.

2 participants