Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image sizes and srcset for intermediate images #2007

Closed
wants to merge 6 commits into from
Closed

Fix image sizes and srcset for intermediate images #2007

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
07307f2
Fix image sizes and srcset for intermediate images
mikeyarce Mar 1, 2021
3beb3f3
Indent with tab instead of spaces
mikeyarce Mar 2, 2021
021f2f8
Fix spacing and comment punctuation
mikeyarce Mar 2, 2021
e29abba
Change `else if` to `elseif`
mikeyarce Mar 2, 2021
7a534f8
Move dimension functionality into separate function
mikeyarce Mar 2, 2021
ed8fe26
Merge branch 'master' into fix-image-size-srcset
rinatkhaziev Mar 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions a8c-files.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,62 @@ function a8c_files_maybe_inject_image_sizes( $data, $attachment_id ) {

return $data;
}
/**
* Filter dimensions for images to return a proper image size and srcset values
*
* @param bool $value The filtered value, defaults to `true`.
* @param string $image The HTML `img` tag where the attribute should be added.
* @param string $context Additional context about how the function was called or where the img tag is.
* @param int $attachment_id The image attachment ID.
*/
function a8c_files_maybe_inject_dimensions( $value, $image, $context, $attachment_id ) {
Copy link
Contributor

@GaryJones GaryJones Mar 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use type declarations on these parameters and return?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! We're not really using most of these since we're just returning true for the value in this filter and we're not returning any of them either, so I wasn't sure if that would be helpful in this case. Would you suggest still adding them? I also wasn't sure if I should pass them all even if we're not using them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're not using them, then there is no need to support them in the function signature, unless a param to the right is being used.

However, since you need $attachment_id, then they all need to be passed.

The non-first types should be known, as they are from core code, but the first type may have been filtered at an earlier priority into a different type, so although it's not being used here, the advantage of using types here is that it becomes an extra check in case some other code is doing something wrong. It's also good for IDEs and static analysis tools, like Psalm or Phpstan

Adding the return type also has no drawbacks.

$vipgo_img_src = preg_match( '/src="([^"]+)"/', $image, $match_src ) ? $match_src[1] : '';

/** This filter is documented in wp-admin/includes/media.php */
add_filter( 'wp_image_src_get_dimensions', function( $dimensions, $image_src, $image_meta, $attachment_id ) use ( $vipgo_img_src ) {
return a8c_files_calculate_image_src_dimensions( $dimensions, $image_src, $image_meta, $attachment_id, $vipgo_img_src );
}, 10, 4 );
return true;
}

function a8c_files_calculate_image_src_dimensions( $dimensions, $image_src, $image_meta, $attachment_id, $vipgo_img_src ) {
$url = wp_parse_url( $vipgo_img_src );
if ( isset( $url['query'] ) ) {
parse_str( htmlspecialchars_decode($url['query']), $query_args );
}

// Let's calculate the aspect ratio of the image:
$aspect_ratio = $image_meta['height'] / $image_meta['width'];

// Is it a Full Size image? We're checking for sizes in query arguments
if (
! isset( $url['query'] ) &&
strpos( $image_src, $image_meta['file'] ) !== false
) {
$dimensions = array(
(int) $image_meta['width'],
(int) $image_meta['height'],
);
// If it's not Full Size, let's use our calculated Aspect Ratio to output proper dimensions according to the size.
} elseif (
isset( $url['query'] ) &&
strpos( $image_src, $image_meta['file'] ) !== false
) {
if ( isset( $query_args['w'] ) && isset( $query_args['h'] ) ) {
$dimensions = array(
(int) $query_args['w'],
(int) $query_args['h'],
);
mikeyarce marked this conversation as resolved.
Show resolved Hide resolved
}
if ( ! isset( $query_args['h'] ) ) {
$dimensions = array(
(int) $query_args['w'],
(int) ceil( $query_args['w'] * $aspect_ratio ),
);
}
}
return $dimensions;
}

if ( defined( 'FILES_CLIENT_SITE_ID' ) && defined( 'FILES_ACCESS_TOKEN' ) ) {
// Kick things off
Expand All @@ -1178,6 +1234,8 @@ function a8c_files_maybe_inject_image_sizes( $data, $attachment_id ) {

// Load the native VIP Go srcset solution on priority of 20, allowing other plugins to set sizes earlier.
add_filter( 'wp_get_attachment_metadata', 'a8c_files_maybe_inject_image_sizes', 20, 2 );
// Hooking into `wp_img_tag_add_width_and_height_attr` to be able to get image_src details needed to inject dimensions.
add_filter( 'wp_img_tag_add_width_and_height_attr', 'a8c_files_maybe_inject_dimensions', 10, 4 );
}, 10, 0 );
}

Expand Down