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] Media Item Menu - Edit Item Images #1345

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

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 8, 2024

Summary

Creates an image section for editing items. Allows for adding, deleting, or editing images for an item. Remote searching is working with all options from server.

Uploading a file is not working. Not sure why. I have the file browser but the data format is wrong.

I also have an issue where the cached images aren't updating after changes.

Screenshots

Screenshot 2024-12-07 at 18 55 18

All Item Images

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-07.at.18.54.32.mp4

Select From Remote Images

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-07.at.18.54.51.mp4

Upload an Image

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-07.at.18.57.40.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

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!

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