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

[tvOS] Episode Selector - State & Focus Handling #1435

Merged
merged 13 commits into from
Mar 4, 2025

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Feb 16, 2025

Summary

This is a very rare occurrence, but I found that if you have empty folder for a season, you can lock the episode selector until you re-enter the ItemView.

The issue is, without episodes, the episode selector does not exist. Because of the FocusGuide, there is no way to go down to Cast & Crew without episodes. Additionally, because there is no set area for episodes, going back to a season that contains episodes, the selector is tiny and shoved outside the screen.

This PR creates a "No episodes" filler text when episodes are not available along with a refresh button in case they want to update this season.

Issue

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

Resolution

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

@JPKribs JPKribs added the bug Something isn't working label Feb 16, 2025
@JPKribs JPKribs requested a review from LePips February 17, 2025 22:29
Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Finally building out this view, I would like it to match the how we do it on iOS: using the same card design for all views. Overall, this will need to consider and work properly with focusing.

@JPKribs
Copy link
Member Author

JPKribs commented Feb 23, 2025

Finally building out this view, I would like it to match the how we do it on iOS: using the same card design for all views. Overall, this will need to consider and work properly with focusing.

I should have checked iOS first I didn't realize you already accounted for empty rows there. I have this version on my latest commit. Is this more of what we want?

Simulator Screenshot - Apple TV 4K (3rd generation) - 2025-02-23 at 13 10 43

@LePips
Copy link
Member

LePips commented Feb 23, 2025

Yes, like that

@JPKribs
Copy link
Member Author

JPKribs commented Feb 23, 2025

Okay, I think I understand the FocusGuide now. I went through iOS and tried to mirror everything that made sense. Overall, it's a fairly minimal change. I've made the following changes:

  1. There is now an EmptyCard when the ViewModel is empty.
  2. I added a ProgressView() to the LoadingCard. It's not a requirement I just think it looks nice. Let me know if that's too busy or hurts performance too much and I can change that to something else like a static "progress.indicator".
  3. The ErrorCard did not work. You could not select it and it was not a normal card. This is now a regular card similar to EmptyCard or LoadingCard
  4. I added padding to the bottom of the selector. This mirrors the same padding that exists on the SeasonsHStack.
  5. Added the ability to focus the ErrorCard to allow refreshing and EmptyCard to allow you to get to the "Cast and Crew" Section.

Regular Season

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

Empty Season:

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

Error Season:

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

Loading Season:

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

@JPKribs
Copy link
Member Author

JPKribs commented Feb 23, 2025

I am done editing this PR unless there is new feedback.

I'm happier with where this is now. While I'm working on this, please let me know if there is anything else that needs to be adjusted in this area. It's a very close mirror to iOS at this point. Since the ...HStack are all their own struct I had to pass in the FocusState. Let me know if that is not the correct approach.

The only item that doesn't have the ability to focus is the LoadingCard. This is because the LoadingHStack is called from another location so I don't know if we want to use a FocusState there as well. Additionally, loading shouldn't exist for that long. Either it will load fully or error fairly quickly.

Only item I have a question on is the LoadingCard can stack a few times. From the videos in my last comment, you can see those stack up to 4 cards. Do we want this or should this only ever have 1 card?

@JPKribs JPKribs changed the title [tvOS] Handle Empty Episode Selector [tvOS] Episode Selector - State & Focus Handling Feb 23, 2025
Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

doesn't have the ability to focus is the LoadingCard

We should have a solution where this entire view can be used regardless of this episode selector, even when loading. Instead of the 2-4 random count of loading cards, which was mainly used as a skeleton view design, having a single card like the other non-content states have to allow better focusing could be a good idea.

@JPKribs
Copy link
Member Author

JPKribs commented Feb 26, 2025

doesn't have the ability to focus is the LoadingCard

We should have a solution where this entire view can be used regardless of this episode selector, even when loading. Instead of the 2-4 random count of loading cards, which was mainly used as a skeleton view design, having a single card like the other non-content states have to allow better focusing could be a good idea.

Sounds good. I've made this change on my latest commit. Everything looks and works as expected but I wasn't sure how to handle this:

>    @FocusState
>    private var focusedSection: String?

...

        VStack(spacing: 0) {
            SeasonsHStack(viewModel: viewModel, selection: $selection)
                .environmentObject(parentFocusGuide)

            if let selectionViewModel {
                EpisodeHStack(viewModel: selectionViewModel, playButtonItem: viewModel.playButtonItem)
                    .environmentObject(parentFocusGuide)
            } else {
>                LoadingHStack(focusedEpisodeID: $focusedSection)
>                    .environmentObject(parentFocusGuide)
>                    .focused($focusedSection, equals: "LoadingCard")
            }
        }

I added a focus state but just for that? I'm not sure that's the right way to go. In testing, it all works correctly but the FocusGuide is still not my best spot. So if there is a different way to handle this please let me know and I can make adjustments.

This is what the LoadingCard and LoadingHStack now looks like:

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

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Looking good, now just some clean up.

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Looks great! Some polish could be that if the loading or error cards and transitions to a new state it will stay focused on the card, instead of it hopping to the seasons.

@LePips LePips merged commit 718ea0f into jellyfin:main Mar 4, 2025
4 checks passed
@JPKribs JPKribs deleted the itemViewCleanup branch March 4, 2025 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants