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.

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

Incorporate layout constraints from ancestor blocks into the sizes calculations #1511

Open
Tracked by #760
joemcgill opened this issue Aug 29, 2024 · 7 comments · Fixed by #1738
Open
Tracked by #760

Incorporate layout constraints from ancestor blocks into the sizes calculations #1511

joemcgill opened this issue Aug 29, 2024 · 7 comments · Fixed by #1738
Assignees
Labels
[Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Type] Epic A high-level project / epic that will encompass several sub-issues

Comments

@joemcgill
Copy link
Member

joemcgill commented Aug 29, 2024

As outlined in #760, the initial work for accurately setting the sizes attribute for image and cover blocks has been released through the Enhanced Responsive Images plugin. While the current implementation effectively handles individual image blocks, it does not account for images constrained by other ancestor blocks. This issue aims to address this issue by focusing on multiple block types, including column, group, row, and stack blocks (alignments only). Enhancements for grid blocks are also being considered (though they are of lower priority due to their complexity and recent introduction).

The primary goal is to improve the sizes attribute across these block types to ensure images are displayed with the correct sizes for optimal rendering across different screen sizes.

@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Aug 29, 2024
@joemcgill joemcgill changed the title Develop a system to incorporate layout constraints from ancestor blocks (e.g., group, row, columns, etc) into the sizes calculation. Incorporate layout constraints from ancestor blocks into the sizes calculations Aug 29, 2024
@joemcgill joemcgill moved this from Not Started/Backlog 📆 to Definition ✏️ in WP Performance 2024 Aug 29, 2024
@joemcgill joemcgill added the [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) label Aug 29, 2024
@joemcgill joemcgill self-assigned this Sep 26, 2024
@joemcgill
Copy link
Member Author

joemcgill commented Oct 10, 2024

@mukeshpanchal27 and I have been exploring this issue over the last several weeks and I wanted to give a quick progress update.

Technical discovery

In order to incorporate layout information from column, group, row, and stack blocks that are ancestors of images, we considered several different approaches.

Use wp_calculate_image_sizes directly

Modifying the wp_calculate_image_sizes filter seems like the most straightforward was to improve the calculation of the sizes attribute. However, one of the key challenges is that this filter does not receive any layout information from the block context, which is crucial for accurately determining the appropriate sizes value for images. Instead, that function is only aware of the img markup itself, not even the figure that wraps the img and provides its own layout alignment.

We created an experimental plugin to demonstrate a proof of concept for calculating the sizes value during block rendering, when more information is available. However, this approach is a bit fragile and will result in a DB query for each image.

Hooking into the block rendering process

In this approach, layout constraints would be passed from ancestor blocks to images using block rendering filter render_block_data. Inspired by how the core gallery block handles this process.

While this approach could work, we quickly found that passing block layout information down to innerBlocks was not performant.

Passing layout information as block context

In this approach, layout constraints would be passed from ancestor blocks to images using block context.

Before each block in a layout is rendered, context can be used to pass information about the current state of WordPress or information from parent blocks to their inner blocks. We can make use of this functionality by hooking into the render_block_context filter to capture information from a parent block—like the parent’s alignment or existence of sibling blocks—and set this as context that can be accessed by the image when the sizes attribute is being calculated.

Some initial experiments with this approach have been promising and looks to be the best approach to pursue at this time.

In the process of working on this approach we ran into the same bug reported in Trac#62046 and have been helping find a fix via WordPress/wordpress-develop#7344 and WordPress/wordpress-develop#7522.

Next steps

At this point, we're planning on moving forward on an implementation based on using the block context approach described above. This will likely be done in multiple PRs that focus on specific aspects of this problem (e.g., passing layout from columns to a nested image, passing layout from a group block, etc).

Any new issues related to this effort should be added as sub-issues of this one to track progress. All PRs should target the newly created feature/1511-incorporate-layout-constraints-from-ancestors branch that has been created for this effort.

@joemcgill
Copy link
Member Author

joemcgill commented Nov 21, 2024

To help with implementation of this issue, I'm listing some use cases we need to solve for

@joemcgill
Copy link
Member Author

@mukeshpanchal27 before we get to far into this, I think trying to reorganize the file structure of the plugin a bit to separate the auto-sizes feature from the functionality to improve sizes calculation could be heplful. I've taked a pass at doing so in #1699.

@mukeshpanchal27
Copy link
Member

@joemcgill Here is different use cases for column block.

  • Add Columns Block

    • Select a predefined layout option for the columns, such as 100, 50 / 50, 33 / 66, etc.
      • If you select a 2-column layout, no width attribute is added to the column blocks.
      • If you select 33 / 66, specific width values are added to the column block.
  • Add Columns Block and Click 'Skip'

    • Clicking the "Skip" button on the layout option adds a 2-column layout without any width attributes in the column blocks. By default, the px unit is selected.
      • Adding a new column in this scenario creates it without a width, similar to the existing column.
  • Transform Image Block to Columns

    • Add an Image block and click "Transform to." This wraps the image in a single-column layout and sets the column's width to 100%.
      • Adding a new column in this scenario creates additional columns with a 100% width, regardless of the number of columns added.
  • Set Columns with Different Units

    • Add a Columns block and assign different units to the column widths.
      • For example, in a 2-column layout, set the first column to 50% width and the second column to 450px.
      • Consider similar cases for layouts with 3 or 4 columns.
  • Column Settings with Group or Columns as Parent

    • When a Group or Columns block is the parent, the same column settings are available for the child columns.

@joemcgill
Copy link
Member Author

Thanks @mukeshpanchal27. This is a good list of ways of creating columns in different layouts. For the purposes of defining the requirements for the the "Use block context to pass layout info from columns:" use case listed here, I think we should focus on what the expected sizes values will be in each scenario based on the block content generated in the use cases you described above.

For example:

Each column block that doesn't contain a width attribute will take up the same proportional space as it's sibling column blocks (e.g., 2 columns: 50%, 3 columns: 33%, etc.). So, if the block markup looks like this...

<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:image -->
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:image -->
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

...the sizes attributes for images should be limited to 50% of the core/columns block, based on its alignment context.

@joemcgill
Copy link
Member Author

As noted in #1701 (comment), when using layout widths returned by wp_get_global_settings() in sizes calculations, it is likely that those values will be set to something other than a px value, and our logic will need to be updated to handle these cases.

According to this documentation, these values can be any valid CSS length value, including absolute values (e.g., px), relative values (e.g., rem, em, vw), percentages, CSS functions (e.g. calc(), clamp()), or even CSS variables (e.g., var(--theme-block-wide-max-width).

For any values that can't be compared directly on the server we'll need to see if we can write sizes values that allow the client to handle the calculations rather than trying to resolve it on the server. For example, in auto_sizes_calculate_better_sizes() where we're comparing the width of an image with the container size to determine which is smaller, rather than doing something like this to resolve a specific px width:

$alignment    = auto_sizes_get_layout_width( 'default' );
$layout_width = sprintf( '%1$spx', min( (int) $alignment, $image_width ) );

We would want to consider something like this:

$alignment    = auto_sizes_get_layout_width( 'default' );
$layout_width = "sprintf( 'min(%1$dpx, $alignment)', array( $image_width, $alignment ) );

@joemcgill
Copy link
Member Author

The first steps have been merged to trunk in #1738 in advance of the upcoming release. I'm reopening this issue for continued iteration on this list.

@joemcgill joemcgill moved this from Not Started/Backlog 📆 to In Progress 🚧 in WP Performance 2024 Dec 13, 2024
@joemcgill joemcgill added the [Type] Epic A high-level project / epic that will encompass several sub-issues label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Type] Epic A high-level project / epic that will encompass several sub-issues
Projects
Status: In Progress 🚧
Development

Successfully merging a pull request may close this issue.

2 participants