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

Navigation and Item Overhaul #492

Merged
merged 34 commits into from
Aug 5, 2022

Conversation

LePips
Copy link
Member

@LePips LePips commented Jul 19, 2022

For anybody who wants to actually look at the code, check out the changes locally and good luck.

Background

This PR aimed to address the issue of episode navigation and make items "more like the Apple TV app" from this discussion. I got most of the way there in this PR. Stopped short out of sanity and will continue over time. Further developments are listed at the end of this summary.

For context, "items" refer to movies, series, episodes, and collections.

iOS Features

There are now three options to present items:

Compact Poster
ios-compactPoster.mp4
Compact Logo
ios-compactLogo.mp4

The above views use a feature of the BlurHash to get the average linear RGB color value and create a gradient to "extend" the image, similar to how Apple does it. It's not perfect since it's literally just the average RGB value and while there is an alternate bottom edge color from the BlurHash, I found the average to work marginally better. I have raised the idea of providing the dominant, prominent, and other colors from images among the JF devs so it may happen and look a lot better in the future. I would use these colors to create gradients and make a lot of improvements with these item views and other places images are used.

Cinematic
ios-cinematic.mp4

The cinematic view is tricky since both the poster and backdrop are not guaranteed at all to be centered. Additionally, the poster will 99% have text on the image so with the logo it won't look as nice. Regardless, it was just an easy implementation and you will eventually be able to change between the poster or the backdrop.

For a series view, there is a Season and Episode selector. Episodes are a single button that will take you to the episode view to see its details. This frankly does not solve the original issue of episode navigation for now on iOS/iPadOS but this will be redone when I get to landscape vs poster stack customization.

Episodes now have their own view:

Episodes
ios-episode.mp4

This view looks different than the other three on iOS only because the content provided by an episode can't fill in the poster or logo.

iPadOS Features

iPadOS now has a single view shared across all items:

Cinematic Movie
ipados-cinematic-movie.mp4
Cinematic Series
ipados-cinematic-series.mp4
Cinematic Collection
ipados-cinematic-collection.mp4
Cinematic Episode
ipados-cinematic-episode.mp4

All items use this view and it looks really nice due to the larger screen and aspect ratio.

tvOS Features

iPadOS now has a single view shared across all items:

Cinematic Movie
tvos-movie.mp4
Cinematic Series and Episode
tvos-series.mp4

For a series view, there is a new Season and Episode selector. When this view is gains focus, it will default to the Seasons row by design as a workaround due to tvOS and the Focus Engine. There are two buttons per episode:
1 - top with the backdrop which will play the episode when selected
2 - bottom with details which will open the episode item when selected

Cinematic Collection
tvos-collection.mp4

This scroll view was very painful to implement because of the lack of tvOS SwiftUI support. This view is kind of fragile, meaning if you try to navigate quickly it will break so please be patient with it.

Other Features and Feature Notes

  • Views that feature a logo will fallback to text if no logo available
  • More attributes like 4k, 5.1, and 7.1 sound
  • Better series episode auto-selection
  • About view for all items with poster and overview
  • Item tag now with the overview

iPadOS and tvOS single option

While iOS has three item view options, iPadOS and tvOS only have a single cinematic view. I planned on also doing a compact view as well for iPadOS and tvOS but thought it was unnecessary as the cinematic view looks great on the larger screens and a compact view wouldn't serve much purpose. However, if a good compact view is presented I may consider it.

Buttons underneath play button

Right now there are only two buttons shown with every view: watched and favorited. If a view has multiple versions, it will also present a third button. It looks quite empty and awkwardly spaced right now but this will be changed in time when the Download and Edit buttons are added as well (iOS and iPadOS). Overall, I think these buttons could have a better design with some kind of material background with a tinted color. Just need to have some kind of background instead of the just the plain icon in most views (probably not in Compact Poster).

Developer Notes

Frankly, listing all of the features above doesn't make it seem like it was a lot of work... but everything that the item view, and I mean everything, was entirely rewritten and this propagated throughout the rest of the app. To start, I refactored some of the main views, BaseItemDto image references, and a lot more I just won't be listing. I learned a lot throughout the evolution of this PR with SwiftUI practices and overall API/object design. This learning caused many refactors over time.

View Declarations

As you can see, all the views are now separated by their type. This is a huge change from my old PR combining all of them. Part of what I've learned over time with good SwiftUI practices is what should be combined or separated. The base of all these views are shared by what scroll view they are implemented in and the content is separated by view. This makes things a lot more declarative for how individual views should look like and separation of concerns for views. I'm being a bit hand-wavy here but by thinking about the view trees in your head you should understand it.

I also really hated having if let __ViewModel = viewModel as? __ViewModel. Looked terrible.

tvOS

tvOS was a PAIN. There is a little to no tvOS SwiftUI support and this past WWDC literally brought nothing new. I attempted many times to wrap some of the TVUIKit views but they literally cannot be wrapped and attempting to make mocks of them in SwiftUI make the app laggy because the FocusState redraws views whenever any view gets a new focus. This was prominent in an HStack for portrait items where these views would be used anyways.

As mentioned above, the scroll view is very fragile. This is because the Focus Engine on tvOS will scroll to a focused view in a very specific way and I was unable to turn off scrolling with any mechanism. I wrote a custom FocusGuide that I thought was soooooo clever but can be a pain to work with because we have to strongly control what is on the screen and what has focus at any time. I also implemented some custom scrolling but even this has issues because of the fading in/out logo/text and when the user has any input during the animation it cancels. By playing around with the Apple TV app I think they use a collection view/table view so I may look at that sometime. No matter what, these views need to be entirely redone in the long term.

Widget Removal

I removed the widget from the project because:
1 - it doesn't work right now, and hasn't for a long time, due to Shared Groups
2 - it was written a long time ago and needs to be redone with our new SwiftUI practices
3 - it had too many errors I didn't bother to fix when I brought over my work from my old branch

I'm being a bit hand-wavy here too but this needed to happen. Trust me.

🧠 dump

  • BlurHashKit was copied over from the original BlurHash source. This was to get the colors from the BlurHash and I wish there was a Swift Package of the implementation. A PR has been on the repo for over a year for SPM support but I wouldn't want to use it anyways as it will also grab all of the other implementations. I might just make the package myself with only the Swift implementation.
  • ImageView was changed a lot and it will change again. The layout of the image and fallback text is incorrect due to how containing views work in SwiftUI and their properties, mainly with sizes and alignments. The following images show the problem:
iOS

The view below has bottom padding, it should have some but not that much.

ios-text-off

iPadOS and tvOS

The views below show the logo/text having a leading or bottom padding. The logo/text should have a bottom leading alignment.

tvos-logo-off
ipados-logo-off
tvos-title-off

This would just take changing how images are displayed to probably the following:

ImageView(... image source ...) { image in
    image
        .frame(...)
        .cornerRadius(...)
        .padding()
} failureView: {
    Text(... item title ...)
        .font(...)
}

I don't know if this would be an entire refactor or just an additional usage right now. A custom placeholder view would also be added instead of always being a BlurHash, especially for logos because they are just a solid colors and don't look nice so the title should be used instead. Also, sources without a blurhash just display a clear color.

  • The BaseItemDto now has a more central and dynamic way to get images.
  • I will be a bit more picky with how things are named. This comes from changing stuff like PortraitImageHStackView, to a few things, to finally PortraitPosterHStack as I learned to mirror SwiftUI naming conventions.
  • Use ScaledMetric to scale views automatically. Right now I manually check for iPadOS/iOS for portrait poster sizing.
  • Create LandscapePosterHStack, landscape analog of PortraitPosterHStack
  • Refreshing the home screen was removed in tvOS. A refresh functionality will be re-added as part of the refactoring the SessionManager to be in the environment. (SessionManager will be changed to a new name too)
  • Collections will have their own kind of page. Instead of listing things horizontally it should be a grid and I need to think about what should happen with the play button. Probably to first unwatched in the list and first if all watched.
  • I really, really, really need to learn the UICollectionView wrappers. I would replace a lot of views with collection views.
  • I did not test with any unaired/missing items so these views may not work properly there, I don't know.
  • Seasons are not accessible. This was deliberate due to time and I thought were low value because who really looks at seasons anyways? They will be re-added later but really only if somebody requests it.
  • The iPadOS and tvOS cinematic views mirror Apple TV a bit with some notable changes. Mainly, the buttons being on the right instead of the left. This was a deliberate design decision due to how I wanted the logo/text on the bottom instead of the top like Apple. This overall created a design that makes us stand out as our own. Not stand out as the "best", but our own.
  • I used nested structs and other type declarations to create namespaces with many of the new views. This has become a good practice and will be used more throughout the app.
  • The item details (like subtitles/audio/filename) are used in some views and not in others. This needs to be refactored.
  • The Series and Episode selector needs to be fixed, most likely refactored, because most times the view loads before the episodes are loaded.
  • I applied a gradient mask to blur views to create nice transitions. However none of these transitions are perfect and need to be redone.
  • I will most likely be refactoring some other things I wrote in here because I'm that type of person. 😢
  • Every time I look at this app, I can see every little thing that needs to happen and keep these notes inside my head. Instead of creating a ton of issues regarding small code changes, I will probably create a Notion page with everything I think about and have that linked to a pinned issue.

Closes #336
Closes #345
Closes #373
Closes #418
Closes #443
Closes #488
Closes #489

In the end, I did not get to everything I wanted to finish in this PR but as you can tell ... it's gigantic. I just picked up whatever I wanted to work on along the way and left a lot of notes of what needs to change. I kind of just went crazy working on this for so long and need to work on other things in the app to break it up, even with the imperfections and regressions. Thank you and good night.

@LePips LePips mentioned this pull request Jul 19, 2022
1 task
@LePips LePips self-assigned this Aug 3, 2022
@LePips LePips added the enhancement New feature or request label Aug 3, 2022
@LePips LePips requested a review from PangMo5 August 3, 2022 22:01
@PangMo5
Copy link
Member

PangMo5 commented Aug 4, 2022

That's awesome! I'll check it out soon when I have time

@PangMo5
Copy link
Member

PangMo5 commented Aug 5, 2022

The layout seems to be broken in the SeriesItemView when selected `compact Logo` and `compact Poster` in iOS.

PangMo5
PangMo5 previously approved these changes Aug 5, 2022
Copy link
Member

@PangMo5 PangMo5 left a comment

Choose a reason for hiding this comment

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

Due to this PR, Swiftfin's UI/UX has been greatly improved, and the development convenience has been increased and the structure of the project has been organized to some extent.
Thank you for your amazing work! 😀

@LePips
Copy link
Member Author

LePips commented Aug 5, 2022

@PangMo5 Could you test this again with your media? I wasn't able to find any definitions that caused the framing to be smaller than the width, which would cause the .background to be smaller as expected. Thank you!

If you look at how the parallax and navbar offset views are created you can see why a .background is necessary for the content at the moment. This could be fixed later on by clipping the parallax header within the defined height that is already passed for the parallax movement computation.

@PangMo5
Copy link
Member

PangMo5 commented Aug 5, 2022

Yeah, now it works as expected 👍

Copy link
Member

@PangMo5 PangMo5 left a comment

Choose a reason for hiding this comment

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

👍

@LePips LePips merged commit a9f09ed into jellyfin:main Aug 5, 2022
@LePips LePips deleted the navigation-and-item-overhaul branch September 4, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment