Sni 6952 video cld poster

pawelphilipczyk-cloudinary commented 1 year ago

Pull request for cloudinary/frontend-frameworks

For which package is this PR?

@cloudinary/angular @cloudinary/react @cloudinary/vue

What does this PR solve?

Adds cldPoster to React SDK to allow using CloudinaryImage as a poster with support for "auto" value for getting that image URL generated directly from the provided cldVid.

Final checklist

[x] Implementation is aligned to Spec.
[x] Tests - Add proper tests to the added code.
[ ] Relates to a github issue (link to issue).

patrick-tolosa commented 1 year ago

Added a few comments, in any case I'm not a blocker for merging, this looks fine, I'd just double check Costa's comment.

patrick-tolosa commented 1 year ago

@lbendavid-cloudinary not sure about analytics here, intuitively I don't think it's something worth measuring.

pawelphilipczyk-cloudinary commented 1 year ago

@const-cloudinary @patrick-tolosa this is now all green, including this new functionality added for Vue and Angular (and with the same tests for each of SDK).

One problem is unsolved but I'm not sure it's a problem, what @const-cloudinary mentioned:

When cldPoster="auto" I'm creating the poster based on originalVideo which alters it.
It is not visible on the video sources' URLs but it's still changing the original video object.
I tried to clone it but it does not work (it's either not usable after cloning or there are still some references deeper not really cloned)
The proper solution would be to have a custom clone() method added to the url-gen's CloudinaryVideo. FYI @mckomo-cl

cloudinary / frontend-frameworks