keystone-classic icon indicating copy to clipboard operation
keystone-classic copied to clipboard

Adds CloudinaryImage(s) fixes, video support (beta), select (beta), opinionated enhancements. #832, #3504, #2699

Open danielmahon opened this issue 7 years ago • 13 comments

Description of changes

CloudinaryImage(s) fixes, video support (beta), select (beta), opinionated enhancements. ~~Fixes folder support for CloudinaryImages. Previously only worked with single image.~~ *This PR has outgrown its original purpose... they grow up so fast 😉 *

  • [x] Adds autoCleanup support to CloudinaryImage(s)
  • [x] Adds public_id format and resource_type to CloudinaryImages lightbox
  • [x] Adds the etag property, which is the MD5 generated by Cloudinary on upload. Useful for local file comparisons.
  • [x] Adds select functionality, allowing for choosing remote Cloudinary Images
    • [x] Update select's UI with preview images
    • [ ] Support CloudinaryImages (plural)
    • [x] Currently shows "sibling" uploaded media with prefix format: [cloudinary prefix]/[list path]/[field path]/
  • [x] Adds video upload/preview support (beta) to CloudinaryImage
    • [x] Allow and support video uploads
    • [x] Generate video thumbnails in admin
    • [x] Adds video icon to thumbnails in admin
    • [ ] Support videos in CloudinaryImages (plural) as well
    • [ ] Allow video playback in admin
    • [ ] Handle video use cases for templates (I only needed uploads for time being)

Questions

Am I wasting my time? 🌳🔨
Should we refactor CloudinaryImage(s) to work as one field type instead of two? Rename field to something like CloudinaryUpload || CloudinaryMedia || Cloudinary?

Screenshots

Supports a mixing of images and video: hydrochem-panel-server

Supports upload or selecting from Cloudinary: hydrochem

Adds image previews, file-types, and paths to dropdown: hydrochem

Supports metadata in lightbox caption: cursor_and_hydrochem-panel-server

Adds "open original" button to lightbox: hydrochem

Related issues (if any)

#832 #3504 #2699

Testing

  • [x] Please confirm npm run test-all ran successfully.

danielmahon avatar Dec 20 '16 20:12 danielmahon

I don't think I'm missing anything here but I haven't looked at the 4.x codebase for a bit so while it should be a simple fix, maybe just look it over.

danielmahon avatar Dec 20 '16 20:12 danielmahon

I just realized that Cloudinary might be getting replaced by the Storage API... is that correct? Are 4.x changes needed to Cloudinary types?

danielmahon avatar Dec 20 '16 20:12 danielmahon

Just added support (without trying to change too much) for autoCleanup removal of single and multiple images.

danielmahon avatar Dec 20 '16 23:12 danielmahon

On another note, I would like to integrate video upload support to this as well, as Cloudinary now supports it.

danielmahon avatar Dec 27 '16 20:12 danielmahon

@JedWatson I've started to add video upload support as well. There aren't THAT many changes to make this work but it does complicate this pull-request a tad. Let me know if you'd like me to handle this a different way. My internal thought was to add video support (behind the scenes) and then maybe later we could do a refactor of CloudinaryImages to something like CloudinaryUpload and try to get it to handle "all the things". I've got a project I'm trying to use this with ASAP hence me throwing these things together, but I'd be down to refactor later. It'd be nice to remove the need for 2 field types.

danielmahon avatar Dec 29 '16 18:12 danielmahon

Is this already merged? I just stumbled over this Issue since the autoCleanup of the CloudinaryImage is currently not working in the latest version. I'm quite new to Keystone but after looking at the code and testing I found that the value is never 'remove' in CloudinaryImageType.js.

moemamoe avatar Feb 22 '17 14:02 moemamoe

Hey @JedWatson, just checking in to see that these changes are still on track. I'm hoping to have select up and running today. Tired of having so many duplicates in Cloudinary ;)

danielmahon avatar May 13 '17 14:05 danielmahon

@JedWatson Added select functionality to CloudinaryImage and UI updates, take a look!

danielmahon avatar May 13 '17 18:05 danielmahon

Is this still in review?

MillerApps avatar Jul 19 '17 23:07 MillerApps

Going to add this to our new PR statuses project, and assign it to someone to look through.

Noviny avatar Sep 22 '17 14:09 Noviny

@Noviny Awesome Ben! I am using this/my branch in a few production (low traffic) projects so after making the changes I needed this PR got a little out of hand and some it might be too "opinionated" about how it handles things. I think ultimately it would be best to refactor the entire CloudinaryImage(s) type into a single type like "Cloudinary" or "CloudinaryMedia" since I've been using it to handle documents/images/videos. I would be happy to work on that once I know you guys are on board with that direction or if another implementation would be better. Let me know. Thanks!

danielmahon avatar Sep 22 '17 16:09 danielmahon

@danielmahon its inspiring that you're still improving this pr. I'm sure it'll be merged soon!

morenoh149 avatar Feb 01 '18 03:02 morenoh149

@danielmahon Thanks, save my day!

gusgard avatar Feb 24 '18 18:02 gusgard

aging-season-9-gif-by-friends

danielmahon avatar May 21 '24 03:05 danielmahon