[iOS & tvOS] Playlists - Library & Item View #1428
Conversation
|
Some QA type info:
|
JPKribs
changed the title
JPKribs
changed the title
There was a problem hiding this comment.
I've added a few comments in a review just to help answer some potential questions looking back through this myself. I think this is ready to go with only 4 questions/preferences that might need discussion. Otherwise, for the most part, this is primarily using existing Views and ViewModels so I feel good about minimizing impact.
- Are we fine with Playlists and Collections both sharing the same UI and ViewModel? If I were to make these separate, they would be very similar so, to me, it made sense to combine.
- Do we like splitting the item rows in Collections/Playlists by
BaseItemKind? Should there be a setting to toggle displaying by type or merged? I have a more in-depth comment for this in the review. - Should the Edit/Delete logic be simplified to lean on Server's permissions or should this continue using the local preferences as well? I have a more in-depth comment for this in the review.
- Is there an easy way to do
LocalizationandPlural Localizationwithout just having 2 localizations?
| } | ||
| } | ||
|
|
||
| extension BaseItemKind: ItemFilter { | ||
|
|
||
| // TODO: localize | ||
| var displayTitle: String { |
There was a problem hiding this comment.
For the items themselves, these should be singular. For the sections I am using, they should be plural. Should I add a pluralDisplayTitle or is there a SwiftUI native way to get plurals without having to make a new var & a series of new strings that are just String+s for most of them?
|
|
||
| await MainActor.run { | ||
| self.collectionItems = collectionItems | ||
| self.listItems = listItems |
There was a problem hiding this comment.
The goal of this is to allow usage of the PlayButton to resume the playlist/collection. In my testing this works well but I wasn't sure if there would be some confusion since "Play" wouldn't necessarily give the user knowledge over what will be played.
I like this functionality personally and like the idea of being able to go into a playlist/collection and just resume at unplayed but I want to make sure this isn't too ambiguous.
| @@ -47,19 +47,43 @@ extension ItemView { | |||
| // MARK: - Can Delete Item | |||
|
|
|||
| private var canDelete: Bool { | |||
There was a problem hiding this comment.
This goes:
If the item cannot be deleted, canDelete is always false. Otherwise, if the item can be deleted, only show the option for Collections & Items to be deleted if the setting is corresponding setting enabled. Since those settings only shows up for Administrators, Playlists are always deletable by their owners.
In hindsight, I might have over-engineered permissions when I added Item Editing/Deleting. It might be best to just use the Item.canDelete to determine if the delete is present, removing the local enableItemDeletion & enableCollectionManagement settings to just rely on this from the server. If they have server permissions, it would be presented to the user.
This would clean some of this up, of the StoredValeus and cleanup a bit of the CustomizeSettingsView at the cost that people wouldn't be able to turn off item deletion on a per-device basis. My original reasoning for adding local edit/delete permissions was tvOS where you might have permissions to delete something but you want to turn that off since a tvOS device is likely shared and could potentially have younger/less tech savvy users.
I'm good to go either way but this logic gets repeats on tvOS and iOS and it's not immediately clear what/why I'm doing what I'm doing here so let me know if there is anything I can do to assist in making this easier to maintain.
| switch viewModel.item.type { | ||
| case .boxSet: | ||
| return enableCollectionManagement | ||
| // As of 10.10, only Administrators can edit playlist metadata/images |
There was a problem hiding this comment.
This one is kind of messy...
For now, we don't have to worry about it but in 10.10, I will need to update the ItemEditorView to have playlist specific logic. Playlists, on 10.10, still cannot have images, overviews, ratings, etc. edited by anyone except an administrator. However, the owner a playlist can always edit the name and the isPublic flag. Those are the only two options for editing by a non-adminstrator so there might need to be a separate 'Edit Playlist' button for just those?
Since those APIs are not available to us in 10.8, this is just inherits the standard item editing logic so this is fine for now. This comment is just to indicate more logic will be required in 10.10.
| ) | ||
| .onSelect { item in | ||
| router.route(to: \.item, item) | ||
| if viewModel.listItems.isNotEmpty { |
There was a problem hiding this comment.
This is repeated for iOS, tvOS, and iPadOS. This section splits Collections and Playlists by their media types. IMO, this is a lot easier for viewing/finding items. The order is still retained by the items in their groups but now it's split by the BaseItemKind. I've provided. a video on tvOS below. I think we would add a toggle for "GroupListsByType" or something but this mirrors Collections for Web so adding a toggle just adds more settings. I'd lean against it but I'm up for whatever makes sense!
Web
Video
Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-02-18.at.09.53.03.mp4
…rwise this looks just about perfect. iOS UI just looks bad. Will need to figure out the best way to present items in the playlist. Should be the last TODO. Otherwise, waiting on: jellyfin#1428
|
I have largely not taken a look at this since I don't want it in 1.3, and want to focus on the fixes for that update, instead deferring to 1.4. |
No worries at all. Part of why I left the PR comments are for me to remember why I did these as well. I'll keep this merge-able with Main but otherwise I'm happy leaving this until 1.4+. |
Summary
Part 1 of 2 for: #609
Resolves: #1461
This PR adds Playlists to Swiftfin. This PR does not allow editing Playlists or adding items to Playlists. I intend to add that functionality to a later PR. This starts #609 but I wouldn't consider #609 resolved until I can add the ability to add/remove items from a playlist.
I found that Playlists and Collections are both fairly similar. When I created the ViewModel for Playlists, it was almost identical to Collections. So, I renamed Collections to Lists since this applies to Collections and Lists now, and I added in a single check to ensure we're not allowing media types that are not supported by Swiftfin. This should have been the case prior since Collections can have Audio, Books, Photos, etc. I've added a check so only Movies, Episodes, Series, and Collections can be in Collections/Playlists. This is a bit of a lie since Collections can have Series/Collections but Playlists cannot but a Playlist handles this by turning the Series/Season/Collections into it's component Movies/Episodes instead. This shouldn't impact us since this is on the Server but it is the only real "quirk" of using a single ViewModel for both Playlists and Collections. It just means a Playlist is more permissive for items that don't exist in Playlists.
I added a check on Playlists/Collections so the PlayButton should be functional instead of being hidden/grayed out. The logic I can using it to find the first item that is NOT played as use that as the play button. If there are no items that are not played, instead just use the first item found. I think it could be argued maybe it should be:
Because the PlayButton works on Collections (and Playlists) I moved both Collections & Playlists to use the CinematicScrollView instead of a custom, just the image.
Finally, for the ItemEditorView, I found that on 10.10 metadata, images, etc. can only be edited by administrators so I have the locked in the same way as standard items. Users with the canDelete are administrators with access to the Playlist and the owner of the Playlist. So, Playlists are deletable from Swiftfin by the qualified parties but there is no ability to edit them except by an Admin. This mirrors web with the exception of a new API for making a Playlist public which we do not have access to on 10.8.
Please find screenshots below. Let me know if there are any questions! In terms of real changes, this can be boiled down to:
Screenshots
The Media View - User Has Playlists vs Does not Have Playlists
Playlist PagingLibraryView
Scrolling a Playlist
Note the multiple ItemTypes of Episodes & Movies:
Scrolling.a.Playlist.mp4
Deleting a Playlist
iOS:
Deleting.a.Playlist.mp4
tvOS:
New Resume/Play Button for Collections
iOS:
tvOS: