-
Notifications
You must be signed in to change notification settings - Fork 98
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.
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
Update wp_calculate_image_sizes
to Reflect Changes in sizes attribute
#1381
Comments
wp_calculate_image_sizes()
to Reflect Changes in sizes attributewp_calculate_image_sizes
to Reflect Changes in sizes attribute
Thanks for creating the issue, @mukeshpanchal27. I agree that it would be best if third party code that calls |
@joemcgill @westonruter I have searched in WPdirectory to see if any themes or plugins use the
As explored in POC #1382, we can't add Let me know if I missed anything. |
The WP Directory search uses Regex, so you need to modify the search a little bit. Looks like there are some uses of the function after all: |
Thanks @joemcgill. Since we don't have a next step for this, we'll move it to the next release. |
@mukeshpanchal27 @joemcgill Just discovered this issue via the doc for further enhancing the If there's layout constraints provided, they could be used by the function and the filter. If none are provided, it would just be an empty array, the function would behave the same it does now, and the filter would also only receive that empty array. If we make this change in Core, I think we would need to define a bit more what the shape of those layout constraints would look like (e.g. which array keys to support etc), but I think it's worthwhile pursuing as indeed this would allow us to make the optimizations in the central function intended for this purpose, rather than in the arbitrary |
This is what i and @joemcgill discussed when we exploring the possible approaches for ancestor blocks definition work. As the beta for 6.7 is coming in few weeks, mean time can we add the new array parameter for
Some other key can be added in the allow list for WDYT? |
@mukeshpanchal27 I think overall what you're proposing makes sense. Though I think we shouldn't introduce this to Core until we also have an idea about what the supported (and documented) keys would be for the If we can figure that out in time for 6.7 Beta, that's great. However, it's not super urgent because we can still work towards that solution even without the Core function adjusted: For example, if we implement our enhanced logic in the plugin, we could use a workaround, e.g. temporarily set a global or something like that which the filter then uses. This would of course be a hack, but okay for this kind of thing where we have a clean path forward for once this would be in Core. |
I'm definitely open to the idea that we may need to modify the signature of However, it's important to consider where this function is called:
wp_filter_content_tags()
> wp_img_tag_add_srcset_and_sizes_attr()
> wp_image_add_srcset_and_sizes() Right now, To work around this, we either need to redesign the way From a plugin perspective, we can likely work around these limitations by adding a filter to the This works, but a more direct approach that modifies core directly would be best. |
Bug Description
As discussed in #1354 (review), the
wp_calculate_image_sizes()
function does not currently account for the changes made to the sizes attribute in PR #1329.We need to update the sizes value in
wp_calculate_image_sizes()
so that any usage of this core function benefits from the improved sizes attribute.Alternatively, we should ensure that the enhanced sizes attribute is applied via the
wp_calculate_image_sizes
filter, so any other code that directly callswp_calculate_image_sizes()
will also benefit from the optimizations.The text was updated successfully, but these errors were encountered: