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] [iOS] Refresh, Edit, or Delete Media Items #1305

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Nov 3, 2024

Summary

The goal is to allow refreshing metadata for a select media item. Additionally, allow for manually editing metadata from iOS. I want to get the refresh working from tvOS but have no intention on allowing tvOS to change metadata text.

Eventually, I want to add the ability to add/edit metadata images, and edit subtitles but that's going to be in a later PR. Images and Subtitles require files to POST API so those are going to be a slower process.

To Do:

  • Allow Refreshing an Item/Show/Season Metadata
  • Finish the MetadataEditorView for ALL BaseItemTypes
  • Fix Community Rating Rounding
  • Localize
  • Validate Studio / Person / Genre / Tags Input Views

Screenshots

New Button (Admin Only) New Button (Admin Only)
Genres Genres
Main Main
Metadata Metadata
People People
Studios Studios
Tags Tags

Parental Warning is giving a weird error but otherwise works.
Studios and Cast need an option to select.
Need to add in 3D format & Original Aspect Ratio.
Need to localize the Metadata sections,
Need to localize all strings in new sections
Need to test Movies, Series, BoxSets once the above are done for Episodes
@JPKribs JPKribs changed the title [WIP] [iOS | tvOS] Edit Non-Image Metadata [WIP] [iOS] Edit Non-Image Metadata Nov 4, 2024
@JPKribs
Copy link
Member Author

JPKribs commented Nov 4, 2024

Hey @LePips no rush at all, but part of the metadata I am trying to wrap my head around allowing edits are Studios, Genres, People, and Tags. These are lists of values and I've gotten a system around editing them but I'm getting stumped on UI stuff again. Since these are edit screens, I made them modal but then I have trouble working around the X button in the top left. This is my current version below:

Editing Simulator Screenshot - Editing view on iPhone 16 Pro, showing editable fields.
Not Editing Simulator Screenshot - Non-editing view on iPhone 16 Pro, showing uneditable fields.
Deleting Simulator Screenshot - Delete option on iPhone 16 Pro, showing swipe to delete action.

Also, I am so sorry how big this one is getting. There are lot of these that are just extensions on existing enums but let me know if there is a good way for me to split these out! I could remove the Studios / Tags / People / Genres and make those their own PR but the whole mess of MetadataEditorView sections all need to kind of go together.

Let me know!

…ir own view. Once these are on their own view I should be done. Likely want feeback on existing two views first to avoid issues
@JPKribs JPKribs changed the title [WIP] [iOS] Edit Non-Image Metadata [WIP] [iOS] Refresh & Edit Metadata Pt. 1 Nov 5, 2024
@LePips
Copy link
Member

LePips commented Nov 5, 2024

I would actually like the Metadata view to be a modal and then the other edit views are pushed. That should probably help with the close button. I have other comments on the data input of some things, but I'll save those for a bit later.

@JPKribs
Copy link
Member Author

JPKribs commented Nov 7, 2024

Current state of this: This PR contains the ability to edit all item metadata or delete an item. All items are functional and work in editing Jellyfin items on the server. I've updated all my screenshots on the main post to reflect this state. However there are some issues I need to work out:

  • Metadata list should update with changes
  • Genre list should update with changes
  • Tags list should update with changes
  • People list should update with changes
  • Studio list should update with changes
  • The People portraits are stretched in a weird way

This PR is frankly massive but there is some room to trim down:

  • Genre & Tags are both [String] so I should have both re-use the same view instead of unique views
  • The ViewModels are functional but messy
  • There is some room to trim down what exists between views

Things that would be cool but I'm not sure what/how to do it:

  • Make the Genre/People/Studio/Tags validate against existing items on the server. So, select an existing lookup item OR create a new item.
  • On 'Refresh Metadata' check the progress and report that back.

@JPKribs JPKribs changed the title [WIP] [iOS] Refresh & Edit Metadata Pt. 1 [WIP] [iOS] Refresh, Edit, or Delete Media Items Nov 7, 2024
@LePips
Copy link
Member

LePips commented Nov 9, 2024

Per my comments in #1296 around the button styling, I would ideally want something similar for that row of buttons on iOS. The same issue with button crowding would still exist here, but luckily this and many other actions might be better on iOS in a Menu on the navigation bar

I've looked at adding more navigation bar items before but I can't seem to find the code for the specific style that I want these buttons to be and how they would change in style how the Apple TV app does it when you scroll... so I'll have to remake that to provide some guidance.

@JPKribs
Copy link
Member Author

JPKribs commented Nov 9, 2024

I can add that to the final version of this! I think edit/delete is better there as well since it's more out of the way. I don't imagine these are functions that people will be using that often so toolbar makes a lot of sense.

One question, this PR is very close to being ready. From there, I am going to break it up into more digestible parts for approval. This is more just a place for me to get my head around everything before I start splitting this up. That all being said, I wanted to check with you if this is the correct way I should be doing my ViewModels for this:

f832203

I tried to model this after ItemViewModel. I'd appreciate some feedback!

I have one outstanding item where I have a dismissCoordinator that is crashing Swiftfin when it's triggered but only for 2 out of the 6 views. Once I have that, I'll start splitting this out for Review!

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.

The ItemDetailsViewModel works looks good to me!


// MARK: - Update Item on Server

func updateItem(_ newItem: BaseItemDto, refresh: Bool = false) -> State {
Copy link
Member

Choose a reason for hiding this comment

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

What would be the reason we would want to refresh locally vs on the server? I see that we do so differently based on the genres, people, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the Genre & Tag are just [String] updating item = newItem just works. However, the person/studio is being input as a name. Jellyfin turns these into full BaseItemPerson/NameGuidPair. I've tried updating it as item = newItem but the list view isn't able to get the image for the person and the Studio doesn't actually link to a Studio object since it's just guid=nil and name=name.

The only reason I'm not updating the Genre and Tag is it felt unnecessary.

The other route for this is to make the addPeople/addStudio view look up the item instead of just inputting from a name. I'd be game to make those lookups on the server but I might need some guidance on what we'd what that view to look like.

@JPKribs
Copy link
Member Author

JPKribs commented Nov 10, 2024

I spun off the first batch of this to #1310.

All of the next steps are done on this PR here but there is still 1 bug that I need to work out so the impact views for that bug are going to be last:

  1. EditMetadataView (I might split the lock metadata options to their own view as well)
  2. EditGenreView
  3. EditTagsView
  4. EditPeopleView [Impacted by routing issue]
  5. EditStudioView [Impacted by routing issue]

@JPKribs JPKribs added the duplicate This issue or pull request already exists label Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants