Create Library Alpha Picker #980
Conversation
|
I have completely refactored how filters are implemented in #905. I'm almost done and adding this work should be easier to implement, so I ask that this work be redone once that's in, apologies.
This is the extent of what we can do given that the server doesn't give a list of section titles. However, that would be a great addition to collection folder/folder
By my n=1 testing of playing with a movie with Korean characters, it does. So as far as I can tell, we have full UTF-8 support for names. |
No worries at all! I'll keep an eye out for when that's ready then I'll take another look at this.
Sounds good. I like the localized letters if they work but I think this means we don't catch Æ types. I'm comfortable with that implementation but I just wanted to make note that other clients are having this conversation. One idea I had after I made this would be 3 API searches.
Getting NameGreaterThan using nameStartsWithOrGreater then removing items that start with that letter... The end result is # == Any non-Localized Letter. I'd be happy to take a crack at that once that new filters are in but it sounds hard to make performative. |
|
#905 has now been merged, so this feature can continue. We don't need to make another class for holding the letters, we only need to add another |
|
First and foremost, #905 resolves my only pain point I've ran into with Swiftfin! The change in the library scrolling is awesome. Literally 0 hitching even on remote loading. It looks like a lot of hard work went into making this and it really shows. I'm excited for when this makes its way to the App Store! Regarding this PR, I'm going to take a look at this tonight/this weekend. I might just nuke my branch and start over with the changes. PR#1 I'll add the filter itself then, with the library changes, I think I need to go back to the drawing board for the AlphaPicker itself. I personally like the idea of mirroring other clients/web with the letters down the side but I'd need to figure out how to avoid cramping into the library. I'll ping once I have something worth looking at! |
|
Any interest in continuing this work? If not, I can implement the letter filter and leave the library picker as later work for you. |
|
Hey, sorry work has caught up to me as of late. I had a chance to review your new filters. It looks a lot cleaner/intuitive but when I took a crack at it I ran into some trouble understanding how to populate the filter values using the section titles (#, A-Z). I want to say I'll be able to knock this out soon but soon could mean early May just with how things are going as of late... I don't want to hold anything back so if this is holding anything back I wouldn't be offended if you did it. Otherwise, I can work on getting this wrapped up soon (May) from my end so I'm not adding to your plate for Swiftfin. Your call! |
|
No worries at all. I just happen to have a lot of time on my hands to get things done and think this would be a great addition. I've implemented the letter filter in the PR below and I would anticipate the picker to be quick when you get to it. You are free to continue this PR or close it. |
|
That sounds great! Thanks for doing that. I think the work that I have for the letter bar should be pretty easy to port over to a new filter once the filter exists. I was just having trouble trying to understand how to populate a filter with a static list of letters opposed to an enum/returned api list. When the letter filter exists, it should only take me an hour or so to point the letter bar to that instead of the old version I was working with. The only quirk I am unsure about is how it will interact with the new library/list view. In particular, with how you are determining the size of posters and how much available screen space there is. But, we can get there when we get there. I'll keep an eye out for the new PR you made I will update this branch, and see about implementing the letter bar. From there, there might be a point where we need to have a discussion about spacing, but I can ping you when I get to that point! |
JPKribs
force-pushed
the
libraryLetterFilter
branch
from
e41ae6b
to
2dace31
Compare
|
@LePips Thank you for setting up the filter! Getting this migrated to the new filter was a cakewalk. Please see the filter below set to the Letter X. Re-selecting X disables the filter. Filter in Action
My next step is I want to re-add a setting to enable, disable, and choose Left vs Right side of the screen. That shouldn't take too long but I wanted to make sure that:
|
|
With this last push, I think I have all of the items I am looking for but there are some rough parts that I want to address before pushing. Completed Items:Created an Alpha Picker Setting
Created New Labels for these Elements (Alpha Picker **Title** so if that changes we can just update the title)
Functioning Filters from Alpha Picker Selection
Flip from Static View to ScrollView if there are more than 27 Characters
Outstanding Considerations
Let me know! |
JPKribs
force-pushed
the
libraryLetterFilter
branch
from
2294c60
to
b2a31db
Compare
…r Bar instead of AlphaPicker. LibraryView isn't where it was in the previous version? New work in PagingLIbraryView?
…y makes more sense than the weird class.enum thing I had going on.
There was a problem hiding this comment.
This looks great! I've only been able to test on a single device in the simulator and after some cleanup should be good to implement. I will come back to it after I do some refactoring on my side with scroll(ifLargerThan:) (or a different modifier) for many characters and may change the spacing/sizes a bit when I get time to tinker.
Swiftfin/Components/LetterPickerBar/Components/LetterPickerButton.swift
Outdated
Show resolved
Hide resolved
…rientation setting when the LetterPicker Enabled status is false. Create new LetterPicker Enabled default (false) and setting to live about the Letter Picker Orientation. Removed Unused variables/constants, and switch from activated flags to isSelected flags. Use the ItemLetter as the datatypes instead of String. Remove all the extentions with inits in them. Cleanpup the contentLetterBarView. Wrap the letterpicker logic with a check for the letterpicker enabled. Re-enable the ignore safe area. Will need to resolve my spacing issue with non-romantic languages still.
|
@LePips I've resolved most of the items you mentioned. I have a questions about some of the items you said I could remove just to make sure I understood. Lastly, the only outstanding issue is this item: #980 (comment) Non-English/Romantic characters create an issue where the scrollbar starts in the safeArea and as a result the letters at the top and bottom cannot be scrolled to for smaller iPhones. iPad and Max iPhones are not impacted but there is the added issue that those devices, I incorrectly turn it into a scroll when a static bar works better and the scroll is not required. So I wasn't sure if this item was more incorporated into a scroll(ifLargerThan:) change or if that would still be impacted. I've included a screenshot. |
…sier but it's the only way to prevent these typechecker errors I've been getting. Turned the LetterPickerBar testing for 27+ to be a static constant since this should not change without restarting the app and this is referenced in 2 places. This should make a conversion to something more scientific later easier. Korean (and other non-English languages) should now fit in the screen appropriately. This LetterPickerBar.isScrolling check onSizeChanged is to get rid of this perpetual loop I get when the language is English.
|
This should now work without filters. This works for non-English/Romantic characters and stays in screen but I think it's a little hacky since I am running the check for non-Romantic but skipping it for Romantic since it seems to loop forever with English. I'm 100% up for changes on that! SwiftUI is definitely my weakpoint here haha. |
Co-authored-by: Ethan Pippin <ethanpippin2343@gmail.com>
…ere is room but this expands out to a scrollview when there isn't room. Taken from: https://stackoverflow.com/questions/62463142/make-scrollview-scrollable-only-if-it-exceeds-the-height-of-the-window. Un-split the PagingLibraryView and made the onChangeSize uncondititional. Functional and working tested on: - iPhone SE 3rd Gen (Scrolls) - iPhone 15 Pro (Eng: Static, Kor: Scrolls) - iPad 13" (Static)
|
Alright! I think I finally have everything cleaned up @LePips. I think there is one final outstanding item that I can see and that's that the position of the scroll is reset when the letter is selected. I've hit a bit of a wall. I know I've resolved something like that in the past but I'm drawing a blank tonight. I can take a look at that another day. The only thing that is new/not suggested by you is the LetterPickerOverflow class to handle the non-romantic characters. This was modeled off of what I read here: https://stackoverflow.com/questions/62463142/make-scrollview-scrollable-only-if-it-exceeds-the-height-of-the-window. Let me know if there is anything there I should be changing. This does the scroll based on screen size now. In testing this was my finding:
|
This feature creates a new filter in the ItemFilters for AlphaPicker. The goal is to (eventually) mirror the functionality of the Jellyfin-Web AlphaPicker which filters the library based on the letter selected. This PR focuses on the creation of the Filter as to split out the underlying Filtering components that exist within the existing ItemFilter / FilterDrawer objects.
Letters for the filter are pulled from the localized language on the device. The filter sets the API Parameter nameStartsWith to the letter selected or uses nameLessThan: 'A' for symbols. This reflects the behavior seen in other implementations of this feature. To assist in this, I have an AlphaPicker class that returns the appropriate nameStartsWith and nameLessThan parameters based on the AlphaPicker.selection.
When testing, I found that the current unstable/development branch of Swiftfin has an issue where the Genre & Filter filters were only referenced in the _getDefaultParameters call. This means that when you updated a genre filter, it would not be reflected until you next hit the requestItems() call. This issue did not exist with the SortBy and SortOrder parameters since these are written during the requestItems() call. To resolve this, I moved the filter objects outside of the defaultParameters since 1) the default call for parameters doesn't need to be filtered and these parameters are being created and set to nil and 2) this resolves the issue and the filters now apply directly after being set. Let me know if there is a better way to handle this!
If this PR is merged, I will work on adding the AlphaPickerBar to the LibraryView to mirror Roku/Web.
Potential Concern: jellyfin/jellyfin-roku#1716
This is a rewrite of something I started lasted year. I chose to take some time to learn Swift better and hopefully rewrite this feature with a little more direction. #852