Skip to content

Conversation

@westonruter
Copy link
Member

In #1596 we need to exclusively look at the desktop group of URL metrics to determine the widest possible width for a given element, excluding widths from narrower viewport groups (e.g. mobile) since they could result in an image size being chosen which would look bad on desktop. This adds a OD_URL_Metric_Group_Collection::get_last_group() helper method to facilitate this. It also adds a OD_URL_Metric_Group_Collection::get_first_group() for parity.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) labels Oct 17, 2024
@github-actions
Copy link

github-actions bot commented Oct 17, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Comment on lines -89 to -94
$widest_group = array_reduce(
iterator_to_array( $context->url_metric_group_collection ),
static function ( $carry, OD_URL_Metric_Group $group ) {
return ( null === $carry || $group->get_minimum_viewport_width() > $carry->get_minimum_viewport_width() ) ? $group : $carry;
}
);
Copy link
Member Author

Choose a reason for hiding this comment

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

This iteration is now eliminated for every video[poster] in the document.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @westonruter, LGTM!

@westonruter westonruter merged commit 43faef8 into trunk Oct 17, 2024
@westonruter westonruter deleted the add/group-collection-first-last-getters branch October 17, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants