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

[WIP] [tvOS] Letter Picker / Filters #1407

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Jan 23, 2025

Summary

Resolves: #333

Takes the same logic from the LetterPickerBar on iOS and implements it to tvOS. Spiritual successor to: #1177

This PR is functional and 'ready' but there is still a lot of discussion that needs to occur before tvOS filters are implemented. This is a precursor to that conversation and, hopefully, somewhat reusable if we end up going another direction.

This PR stands as more of a placeholder for the logic opposed to what the final version of this will look like. I've rebased this on Main again so it should be up-to-date compared to my old version of this PR which was very far behind without a reasonable path to rebase.

I have a current working version of this that can be seen below:

Current Version
Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-06.at.12.28.30.mp4

Outstanding Issues

  1. On Main, tvOS PagingLIbraryView jump their focus to the top of the page whenever a new page of data is loaded. Since the Letter Picker is the tallest item, this is now capturing the focus. I think this is more of an issue with the PagingLIbraryView and knocking out that TODO. I am not sure if this is a package issue where the Focused item just gets lost when new items are added in?

  2. If you are hovering a letter that is above, below, or between posters, it cannot jump to them. Instead, you have to go back up to a letter that in even to a poster to move the cursor back to the posters. I will need to figure out the FocusGuide to use this appropriately but this should be doable.

Outstanding Discussions

There is a whole conversation about tvOS UI I know LePips wants to have. This should provide some clarity on what we want this to look like so we aren't reworking the same view multiple times to no benefit. This PR's work should hopefully be re-usable when that conversation is had but this is likely not our ideal version of this. I personally have a poor eye for UX/UI so I want to leave plenty of room for news items/revisions. However, this PR introduces the FULL FilterViewModel so any filter logic we need to do should be feasible.

@JPKribs JPKribs added enhancement New feature or request low-priority labels Jan 30, 2025
@JPKribs
Copy link
Member Author

JPKribs commented Feb 7, 2025

This was the original implementation of this. I am not advocating for this layout but more of the only idea I had for this on tvOS:

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2024-08-07.at.15.10.01.mp4

@JPKribs
Copy link
Member Author

JPKribs commented Feb 27, 2025

This was idea 2:

Letter bar optionally on one side and this on the other. I've included my VERY WIP version on my latest commit.

trim.C514363D-DCF2-45ED-A84C-F336407B52C4.MOV

…ilter drawer is "done" but the spacing needs work. Next work be adding the Filter views that pop up and allow you to select them. Currently, selecting anything except for "Reset" will just crash Swiftfin since the views don't go anywhere.
@LePips
Copy link
Member

LePips commented Mar 1, 2025

Apologies that I haven't been active lately and really active at all on these draft PRs, but I do really enjoy that design!

@JPKribs
Copy link
Member Author

JPKribs commented Mar 1, 2025

Apologies that I haven't been active lately and really active at all on these draft PRs, but I do really enjoy that design!

Thank you! I'm a fan too! If we end up going another direction I wont be offended but I think I have this in a POC state now. I spent a lot of times going through other apps on tvOS only to realize that most apps don't actually have filters so this was just me throwing things at the wall. I'm happy with my progress so far but, I definitely have some room to work out some of the finer details. Most notably spacing:


Trailing

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-02-28.at.20.41.58.mp4

Leading

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-02-28.at.20.41.23.mp4

TBH, I have no idea why that spacing variables between the two of them. It SHOULD be identical. But, I got the expanding part to work bidirectionally so I think this is a decent start!

@JPKribs
Copy link
Member Author

JPKribs commented Mar 1, 2025

Latest Commit

This PR is fully operational!

Filters now actually work. There are some questionable decisions in logic that should be pretty quick to resolve. These are in FilterViewModel and ItemFilterCollection where I added some logic to NOT check itemTypes for hasFilters checks. Also, on resetting filters, preserve the itemTypes to prevent turning Movies or TV Shows into Everything on accident.

These changes are purely proof of concept and kind of heavy handed IMO. I think there is a cleaner solution but for now it works. Filter pages use the same systemImageable I am using for the buttons. Otherwise mirrors iOS.

IF this is the UI we like, I would still need to clean up all of the spacing AND I'd need to work out the FocusGuide because right now these buttons do not utilize focus correctly. Other than that, plus considerable polish, I think this is ready. As far as POC, I think this is fully ready!

I'm 100% open to feedback! If people don't like this or we want to go another way, I won't be offended at all. Hopefully, this PR has some groundwork that can be recycled back into our best version of this!

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-02-28.at.21.26.16.mp4

@JPKribs
Copy link
Member Author

JPKribs commented Mar 2, 2025

I think I made it worse? This is my working version. The idea being the spacing is always consistent and the filters kind of exist 'over' the content. There are scenarios where this gets a little hard to see so maybe not.


List

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-01.at.23.41.28.mp4

Grid

The grid video is like 10x bigger so I had to shrink it down to only cover this part of the screen.

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-01.at.23.54.57.mov

I had the idea of moving the content over to accommodate the expanded filters and it ended up looking like this:

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-01.at.23.59.05.mp4

I like the idea of putting this in the margins like the LetterPickerBar to try and "even" out the view. Additionally, it prevents us from having the toolbar + filters all at the top, which pushes the content down. I think the margins looks good but spacing is my biggest enemy here...

Finally, I'm not sure what the SearchView version of this looks like. I made this a ViewModifier to mirror iOS. I can add it to the SearchView but it looks a lot more out of place. The SearchView looks a lot better with Horizontal filters at the top. Which, at that point, we should just do that for the libraries as well...

@JPKribs
Copy link
Member Author

JPKribs commented Mar 3, 2025

Last Update for a while

I am going to take a break work shopping this and focus on some of the other PRs I have open (primarily playlists and maybe track selection). This is what the PR currently looks like. The latest change was adding some background to the buttons when focused. I see other apps use this layout (Netflix) so it looks fairly standard:


Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-02.at.21.11.28.mp4

There is a LOT of filler work in this PR. Primarily, I wanted to get a version of this to test out its appearance and get something that I thought looked good. For a more final version, I would need to work on getting some of the spacing / logic more dynamic instead of the static values in there. If this is a route people like, I'd be happy to continue but for now I am going to focus on other PRs.

@JPKribs JPKribs marked this pull request as ready for review March 3, 2025 04:48
@LePips
Copy link
Member

LePips commented Mar 9, 2025

I do like the newer change that the filters are more like a "drawer" so that it actually doesn't take up much space.

This does differ from the original idea I had in mind where it is just a row above the content, overall similar to iOS, but I think this side-by-side design works better for the platform. However, Search would look better with all of the buttons in a row below the search bar.

We could eventually create the top row design but that would require finally fixing the tab bar to disappear on scrolling:

// TODO: Figure out proper tab bar handling with the collection offset

As well how to make that row of elements scroll along with the grid view.

@JPKribs
Copy link
Member Author

JPKribs commented Mar 9, 2025

I do like the newer change that the filters are more like a "drawer" so that it actually doesn't take up much space.

This does differ from the original idea I had in mind where it is just a row above the content, overall similar to iOS, but I think this side-by-side design works better for the platform. However, Search would look better with all of the buttons in a row below the search bar.

I'm fine going either direction. I found the same issue where Search does not look good with the vertical filters. So, we'd need a horizontal filter regardless. IMO, it makes sense to only have, and support, one filter layout. So, I'd be game to look at that direction instead.

I'd be happy to look at that as well. If you have something else in mind and want to pursue I don't want to complicate it and get in the way. Otherwise, I didn't anticipate how much time I'd be sitting at airports so I've found a lot more free time to look at things like this if that would helpful.


Example:

This is my latest commit. I have a searchFilterBar and libraryFilterBar. One Vertical and one Horizontal but they use the same buttons.

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-03-09.at.21.13.51.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low-priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tvOS - Search, Sort, Filter
2 participants