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

Refactor PosterButton and libraries, good UICollectionViews, proper orientation handling, and more #905

Merged
merged 63 commits into from
Mar 11, 2024

Conversation

LePips
Copy link
Member

@LePips LePips commented Nov 9, 2023

Just another huge PR that meant to start small and snowballed into a huge refactor. Honestly, I don't remember everything that I did, but we're here now. 🤷 I'm only going to really highlight user facing features below.


Refactor Libraries and HStacks

The initial focus of this work was finally fixing the underlying UICollectionViews for libraries. I did not know much about how these views worked prior to this work but I was able to get comfortable with these old and powerful views in depth. My projects CollectionVGrid and CollectionHStack replace the libraries and HStack implementations. These are far more performant in both memory and developer use.

Before (15 Pro)

Layout bugs
Different layout on 15 Pro Max

The layouts are now universal between all iPhones and slightly different on iPads. iPhones have the same number of columns and iPads determine their column count with the (rough) calculation: floor(screen width / minimum view width). I've been able to make layouts that allow a lot of customization and I am still working on some for some other views (like the item "About" view, which now looks a bit out of place).

After (15 Pro)

iPad layouts still need some attention, but they look better too.

Layout Menu

There's a new menu for customizing the layout within the library. The list view now allows selecting between the landscape and portrait videos. On iPadOS, you can also select the number of list columns in [1, 3].

Layout Menu

Simulator Screen Recording - iPhone 15 Pro - 2024-03-10 at 19 02 38

New Issues

  • In some contexts it doesn't make sense to use certain view types. The issue today is that it for people it doesn't make sense to use the landscape posters, but the library will allow it and the views will just be in the empty state. This can also be held true for episodes, folders (maybe), and in the future books and square views like albums. So a mechanism to only allow a subset of possible view types needs to be (re-?)implemented.
  • For the horizontal stacks if someone scrolls fast enough the views will be animating their layout
  • There are some quirks with the combination of UICollectionView and SwiftUI that cause the views to sometimes redraw themselves (I think). This is evident by some posters going back into the blurh hash view.

Filters

This work was all under-the-hood and future filters are quite easy to implement. However tags and years were also added.

Nuke Major Update

The Nuke package had a major update that changed how our underlying image views work entirely. There is an outstanding issue that I wasn't able to figure out on item views where the header image isn't being clipped correctly and will slide in.

Image bug

Simulator Screen Recording - iPhone 15 Pro - 2024-03-10 at 19 21 37

Tidbits

  • the failure state for poster images are now an icon depending on the item type
  • the item overview view is now a larger font and the truncated text view is now selectable on the entire view instead of just "See More"
  • Search and Media will now show expected loading and error views
  • folders now nest as expected
  • various other refinements both visible and under the hood

tvOS

tvOS is again an under-developed-for platform. My main focus is on iOS right now but I still implemented the new collection views. There is a weird issue though in my CollectionVGrid where the spacing isn't uniform, so that still needs to be worked on. Besides that, there aren't any notable features worked on.

Future Work

A lot of work done here lays the foundation for many other library/refresh issues to be closed. That will be my next focus alongside a better user management system. Additionally, the other views that still use the old CollectionView need to migrate over to CollectionVGrid.

I've now found a style that I really like in SwiftUI that has allowed me to write things a bit quicker and potentially refactor easier. I've also been commenting a lot more and added more TODOs for future work and new bugs.

Issues Closed

(or have been tested with this work and no longer are an issue)

@LePips LePips marked this pull request as draft November 9, 2023 02:22
@LePips LePips marked this pull request as ready for review January 16, 2024 06:01
@LePips LePips marked this pull request as draft January 16, 2024 06:01
@LePips LePips changed the title Refactor PosterButton and good UICollectionViews Refactor PosterButton, good UICollectionViews, and better Portrait/Landscape orientation handling Jan 25, 2024
@LePips LePips changed the title Refactor PosterButton, good UICollectionViews, and better Portrait/Landscape orientation handling Refactor PosterButton and libraries, good UICollectionViews, and better Portrait/Landscape orientation handling Jan 26, 2024
@LePips LePips mentioned this pull request Jan 26, 2024
@LePips LePips changed the title Refactor PosterButton and libraries, good UICollectionViews, and better Portrait/Landscape orientation handling Refactor PosterButton and libraries, good UICollectionViews, proper orientation handling, and more Mar 9, 2024
@LePips LePips marked this pull request as ready for review March 11, 2024 01:51
@LePips LePips requested a review from PangMo5 March 11, 2024 02:34
@PangMo5
Copy link
Member

PangMo5 commented Mar 11, 2024

I'll check it out soon.

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

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

✅ yep those are changes

@LePips
Copy link
Member Author

LePips commented Mar 11, 2024

@PangMo5 there is no pressure and I can answer any questions. I'll really try to kick this habit of these larger multi-feature encompassing PRs. (I've thought of how I can refactor the video player gestures, but at least that's local to that feature)

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.

Don't feel pressured by make a large PR. It's fine for me
And It's better if your good work is distributed small and fast, but if anythings helps the project, don't worry about it and do your work.
Thanks for the great work.

@LePips LePips merged commit a645444 into jellyfin:main Mar 11, 2024
3 of 4 checks passed
@LePips LePips deleted the finally-some-good-ish-collection-views branch April 3, 2024 05:16
@LePips LePips added the enhancement New feature or request label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment