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

[iOS] Media Item Menu - Edit Item Images #1345

Open
wants to merge 62 commits into
base: main
Choose a base branch
from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 8, 2024

Summary

Adds an image management view for editing Movies, Shows, Episodes, Season*, or People*.

  • Seasons & People don't have an ItemView so there isn't a place where you can select them to edit but they should work if there is every a good view to edit them from.

Features

• View all images associated with a media item.
• Add, delete, and edit images for media items.
• Delete existing images.
• Upload a remote image by searching and selecting from Jellyfin Server's configured sources.
• Upload new images from device files.
• Upload and crop new images from device photos.

Demo

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-05.at.18.05.02.mp4

- Upload image isn't working
- Only a single image is shown per section. Need to make this the HCollection of all images for the group
@JPKribs
Copy link
Member Author

JPKribs commented Dec 8, 2024

@LePips Absolutely no rush on this at all. I'm still a ways away from being ready but let me know if you have any feedback on the UI! Not in love with anything I've done so far but I don't have a great sense of what this should look like. The edit button on the poster is because you can have more than one banner, etc. so I make it so you edited at that level opposed to from the section as a whole.

The remote image section, with the option from remote sources from the server, is paginated so it only loads in 50 at a time. Web is only 25 so I can limit that further if we want.

Functionally, I think my only issue I'm not sure of is I am using item.imageSource(imageType) to get the image based on the type but that only every gives me index 0 so items like banners only let you edit/delete/upload to the first item which, if possible, I'd like to make this usable for any index.

Finally, I'm trying to upload from a file. I've got it so it picks the file using the file picker and I have the upload API working right but I think the formatting for the image is off? I'm going from a local 'URL' to a file path on the device -> Data -> UIImage to run the following:

    private func uploadImage(_ image: UIImage, index: Int? = nil) async throws {
        guard let itemID = item.id else { return }

        var contentType: String
        var imageData: Data
        var request: Request<Void>

        if let pngData = image.pngData() {
            contentType = "image/png"
            imageData = pngData
        } else if let jpgData = image.jpegData(compressionQuality: 1) {
            contentType = "image/jpeg"
            imageData = jpgData
        } else {
            logger.error("Unable to convert given image to png/jpg")
            throw JellyfinAPIError("An internal error occurred")
        }

        if let index {
            request = Paths.setItemImageByIndex(
                itemID: itemID,
                imageType: imageType.rawValue,
                imageIndex: index,
                imageData
            )
        } else {
            request = Paths.setItemImage(
                itemID: itemID,
                imageType: imageType.rawValue,
                imageData
            )
        }

        request.headers = ["Content-Type": contentType]
        _ = try await userSession.client.send(request)

        try await refreshItem()
    }

… because I think that's better. Spacing on the add screen is still all wrong but we're getting closer
TODO:

- Spacing for remote portrait images is wrong & cramped
- Upload image from file browser never works & produces 400 error
- Show all images for an item.imageType opposed to just the first
- Setting image works but produces a 400 error
- Error alert looks bad
@LePips
Copy link
Member

LePips commented Dec 11, 2024

For each grid, we can make assumptions on what the shape of the images are going to be and have the layouts match accordingly. ie: primary is portrait, backdrop is landscape, etc. While anyone can upload arbitrary images, users going against the grain isn't a concern.

For the image contents, we can instead show nothing. When an image is selected, lets push to a confirmation view that shows the larger image with the size and metadata source details.

@JPKribs JPKribs changed the title [iOS] Media Item Menu - Edit Item Images [WIP] [iOS] Media Item Menu - Edit Item Images Dec 11, 2024
Selecting a Remote image is now working without error and works consistently!

Upload a local file is still broken

Item types with multiple images is working as intended now!

Overriding an image on index doesn't seem to work but it doesn't work for Web either so........

UI is way more jank but the hard parts are getting solved!
@JPKribs
Copy link
Member Author

JPKribs commented Dec 23, 2024

@LePips Still low priority but I'd love some feedback on this UI! I'm pretty close to being ready for a review. From my issues below:

1 seems resolved
3 seems to be have gone away without me changing anything
2 I might have to move to another PR. The issue I'm getting is 500 server side so it might be SDK? Might be server? Could be my images? Whatever it is, I'm hitting a wall so I might just look at this at a later date since the RemoteImageSearch portion of this should be complete.

As a note, some files are showing up as "new" but actually just moved. Primarily, organizing ItemEditor I think this got weird when I merged with Main. Moved Metadata views to one folder and Image views to another. Identify views are already in their own folder.


This is still In-Progress but this is my current working version:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-22.at.18.34.29.mp4

I still need figure out the following:

  1. When deleting an Image, if there are multiple Images for that ImageType, the order of the items shuffles around in a jarring way.
  2. The UploadImage + icon doesn't actually work. It always returns a 500 error even though I mirrored UserProfileImagePicker so I need to understand that all better:
Screenshot 2024-12-22 at 18 41 36
  1. At the end of the AddItemImageView scroll, the last image always explodes like this:

Simulator Screenshot - iPhone 16 Pro - 2024-12-22 at 18 46 54

Other than that, I think this is almost ready!

@JPKribs JPKribs requested a review from LePips January 12, 2025 08:57
@JPKribs
Copy link
Member Author

JPKribs commented Jan 12, 2025

I feel good about this current state. There's nothing that stands out to me. My only question is should we make a generic ListRowDeleteButton? I think we have delete buttons that look just like this in some other places as well?

@LePips
Copy link
Member

LePips commented Jan 17, 2025

generic ListRowDeleteButton?

No, as I've left a comment on ListRowButtonStyle that it should take into consideration the configuration.role. When ButtonStyle.destructive is used we would style it like we currently do the delete buttons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants