Adds sources information to the attachment media details of the REST response #224
Conversation
eugene-manuilov
added
[Type] Enhancement
eugene-manuilov
requested review from
adamsilverstein and
getsource
as code owners
mitogh
added
the
[Plugin] Modern Image Formats
mitogh
left a comment
mitogh
left a comment
There was a problem hiding this comment.
Great work on this one, few minor comments/questions.
modules/images/webp-uploads/load.php
Outdated
| if ( empty( $metadata['file'] ) ) { | ||
| return $response; | ||
| } |
There was a problem hiding this comment.
Curios about the need for this conditional if the file key is not accessed in this function.
There was a problem hiding this comment.
Just a concise way to ensure that the metadata array contains data that belongs to an image.
There was a problem hiding this comment.
I'm not sure if it follow that part, I think this sectionof code is not required due you already have this section in place few lines below:
if ( ! isset( $data['media_details']['sizes'] ) || ! is_array( $data['media_details']['sizes'] ) ) {
Which already covers that scenario.
There was a problem hiding this comment.
I agree that check here looks unnecessary. A more appropriate check if we want to avoid running the following logic unnecessarily may be empty( $metadata['sizes'] ) for example.
There was a problem hiding this comment.
@mitogh @felixarntz ok, this check is removed. 👍
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@eugene-manuilov Nothing to add from my end other than what @mitogh has already commented on. Other than that, this looks solid 👍
Co-authored-by: Crisoforo Gaspar Hernández <hello@crisoforo.com>
Co-authored-by: Crisoforo Gaspar Hernández <hello@crisoforo.com>
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
One minor comment, but overall lgtm!
modules/images/webp-uploads/load.php
Outdated
| if ( empty( $metadata['file'] ) ) { | ||
| return $response; | ||
| } |
There was a problem hiding this comment.
I agree that check here looks unnecessary. A more appropriate check if we want to avoid running the following logic unnecessarily may be empty( $metadata['sizes'] ) for example.
Summary
Fixes #167
Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.