remove self hosted video player and use embedded for better handling. #360
Conversation
php/class-media.php
Outdated
| * | ||
| * @return array | ||
| */ | ||
| $transformations = apply_filters( 'cloudinary_transformations', $transformations, $attachment_id ); |
There was a problem hiding this comment.
What do you think if we move this filter just before we return? We would need to use $overwrite_transformations, but it would give us more control.
As a general rule, I like to see filters either early on or the latest possible.
php/class-media.php
Outdated
| public function apply_default_transformations( array $transformations, $type = 'image' ) { | ||
|
|
||
| // Allow filter to bypass defaults. | ||
| if ( false === apply_filters( 'cloudinary_apply_default_transformations', true ) ) { |
There was a problem hiding this comment.
What do you think if we send the $attachement_id to apply_default_transformations() and use it in this filter to have more granular control on this bypass?
pereirinha
left a comment
pereirinha
left a comment
There was a problem hiding this comment.
I left you a few minor comments.
| public function apply_default_transformations( array $transformations, $attachment_id ) { | ||
| // Allow filter to bypass defaults. | ||
| if ( false === apply_filters( 'cloudinary_apply_default_transformations', true ) ) { | ||
| if ( false === apply_filters( 'cloudinary_apply_default_transformations', true, $attachment_id ) ) { |
There was a problem hiding this comment.
@DavidCramer Can we add documentation here to this filter?
php/media/class-video.php
Outdated
| unset( $attributes['poster'] ); | ||
| } | ||
| // Add the player version to use. | ||
| $params['vpv'] = '1.5.1'; |
There was a problem hiding this comment.
Couldn't we use here PLAYER_VER instead?
php/class-media.php
Outdated
| $transformations = $this->get_transformations_from_string( $asset['url'] ); | ||
| if ( ! empty( $transformations ) ) { | ||
| $asset['sync_key'] .= wp_json_encode( $transformations ); | ||
| $asset['sync_key'] .= wp_json_encode( $transformations ); |
There was a problem hiding this comment.
@DavidCramer do you want to fix this alignment? :)
pereirinha
left a comment
pereirinha
left a comment
There was a problem hiding this comment.
This looks great.
Thank you.
No description provided.