Media Library #4: Editing

[frosty]

This PR introduces editing to the media library.

To test:

• Open a media item from within the library.
• Tap the title, caption, or description fields to edit them.
• Once you've edited any of those fields and returned to the media item view controller, the navigation bar will show Cancel or Save.
• Cancel should revert the changes, Save should save them.

Needs review: @kurzee

[kurzee]

Same as #6858 (comment), except that if the user is on a editing detail view, we only popped one controller to the image detail.

[kurzee]

An interesting case:

1. Open Media, select an image for the detail.
2. Open the Editor, Picker, Media Library.
3. On the web, change a field for the media item's details.
4. Refresh the picker's media library, close out.
5. Notice the detail from the Media Library shows the old data.

[kurzee]

@frosty everything looks pretty good! Simple enough. Just the two comments, but I also wonder if we should go ahead and add in alt-text support? Or also the direct URL field to the image, maybe for a separate PR.

[frosty]

Thanks @kurzee! I've pushed up change to properly handle popping the detail view controllers when things change. The media library VC itself now takes care of things, no matter how many detail views are pushed on.

Regarding the other issues:

An interesting case...

So there seem to be two blockers for fixing this.

1. The change observers for the media library currently only report when items have been inserted / removed, not when they're changed. There's a comment that indicates that this behaviour is like it is to avoid a bug, so I'm loathe to change it without more investigation: https://github.com/wordpress-mobile/WordPress-iOS/blob/develop/WordPress/Classes/ViewRelated/Media/MediaLibraryPickerDataSource.m#L377
2. We could also just refresh the detail view on viewWillAppear:, but there's currently a bug where presenting the editor doesn't call the will / did appear / disappear methods on the view beneath it (due to the .overFullscreen modal presentation mode of EditPostViewController.

Because of these, and the fact I think it's a relatively uncommon case, I think we can probably leave this for now?

I also wonder if we should go ahead and add in alt-text support?

It looks like this isn't supported by XML-RPC, which is I think why we didn't have support yet. Adding alt text support would require updating the Media managed object and I think as you say it can be a separate PR.

also the direct URL field to the image

Do you think that's still necessary if we have the copy URL functionality already?

Finally: turns out that XMLRPC doesn't support editing / updating image metadata. I've pushed a change to only show editable fields if you're looking at a dotcom site.

[kurzee]

I'm thinking this should be something like [self supportsRestApi] && [self isAdmin];? As we also lose support for self-hosted over Jetpack with just .com.

[frosty]

Thanks, that makes sense. Technically isAdmin isn't right, as you can edit metadata as an author or editor – but we don't currently have any way to detect that, so let's stick with admin for now.

[frosty]

@kurzee I've pushed up a change to address this. I've also disabled deletion for non-admins.

[kurzee]

@frosty would we also need hide the Media section for non-admin users?

[frosty]

I'm not sure what the correct solution is there. It seems that on the web, everybody but contributors can view the media library at the top level. I just checked on the app, and our syncMediaLibrary call works for contributors too. I also appear to be able to view the media library content in the editor on the web as a contributor.

So either we don't let editors and authors see it (but they should be able to), or we let contributors see it (and perhaps they shouldn't be able to). I think we can probably let everyone see it, as it's visible in the editor for everyone in Calypso.

[kurzee]

I think we can probably let everyone see it, as it's visible in the editor for everyone in Calypso.

👍

Because of these, and the fact I think it's a relatively uncommon case, I think we can probably leave this for now?

👍

Do you think that's still necessary if we have the copy URL functionality already?

I think it's a bonus, but an obvious field that I could tap to get the copy UIMenuController button on would be awesome. I suppose also seeing the direct path could be handy for self-hosted. Probably a separate bonus PR for later.

when ready!