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

Local feeds: add sort option for file name #5629

Merged

Conversation

shombando
Copy link
Contributor

@shombando shombando commented Jan 2, 2022

Background

This PR adds the ability to sort local files by filename as described and discussed in PR #4610. I've refactored the code written by @widlok based on the feedback from @ByteHamster. I brought in @widlok 's commits as is to ensure history and credit is maintained but as addressed in the previous PRs his commits do not have an email associated, and I wasn't able to find one on their profile. If their email is known, I'm happy to do an interactive rebase and add them author on commits prior to refactor.

Closes

Closes #4597 Local feeds: add sort option for file name

Testing

  • Devices
    • Physical Pixel 5a running Android 12
    • Virtual Pixel 4a running Android 11
  • Scenarios
    • Local feeds show sort by file name options in addition to other options
    • Regular podcast feeds only show regular options
    • All sort options work properly and are remembered on revisiting the view

Copy link
Member

@ByteHamster ByteHamster 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 working on this! I have another round of feedback :)

@shombando
Copy link
Contributor Author

Thanks for the great feedback @ByteHamster, I've made the changes requested and just have one question on preference (above). Happy to make any other changes / conform to style as requested!

@@ -90,6 +98,11 @@ private static int duration(@Nullable FeedItem item) {
return (item != null && item.getMedia() != null) ? item.getMedia().getDuration() : 0;
}

@NonNull
private static String itemLink(@Nullable FeedItem item) {
return (item != null && item.getLink() != null) ? item.getLink() : "";
Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense to convert it to lower case. Otherwise, "Bxxx" will be shown before "axxx".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call @ByteHamster, I've made the change and tested it. I also took the liberty to modify the syntax in the switch-case to match the rest of the cases. I was a bit apprehensive to change things that are "not mine" but getting more comfortable doing the sensible thing. Thanks for all the patient coaching.

Input files:
Screenshot from 2022-01-03 17-00-24

Default sort when first adding local feed:
Screenshot_1641250673

File A to Z sort:
Screenshot_1641250688

Let me know if there are other changes. Cheers!

@ByteHamster ByteHamster merged commit 8568226 into AntennaPod:develop Jan 4, 2022
@ByteHamster
Copy link
Member

Thanks! Will be released in AntennaPod 2.5.0

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.

Local feeds: add sort option for file name
2 participants