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

Allow theme to control sizes attribute for wide/full images #6131

Closed
mor10 opened this issue Apr 11, 2018 · 15 comments
Closed

Allow theme to control sizes attribute for wide/full images #6131

mor10 opened this issue Apr 11, 2018 · 15 comments
Labels
Customization Issues related to Phase 2: Customization efforts [Feature] Media Anything that impacts the experience of managing media [Feature] Themes Questions or issues with incorporating or styling blocks in a theme.
Milestone

Comments

@mor10
Copy link
Contributor

mor10 commented Apr 11, 2018

Issue Overview

Theme authors need the ability to control the sizes attribute of images added based on their alignment attributes to ensure the browser knows the displayed width of all images and uses the appropriate image from the srcset list. Traditionally this was done using wp_calculate_image_sizes but within this context there is no way to know what displayed width any particular image in a view has. All we can know is what template is being used.

What's needed is a method to supply sizes attributes to individual images based on their alignment (left / right / center / wide / full).

Steps to Reproduce (for bugs)

  1. Add image to post.
  2. Filter sizes attribute using wp_calculate_image_sizes
  3. Same sizes attribute is applied to all images, regardless of alignment.

Expected Behavior

Given a post with three images:

  • one center aligned (full-width to the regular content width, which we know)
  • one wide aligned which will take up some CSS-defined width in addition to the regular content width (usually 50% of the remaining width on either side of the content)
  • one full aligned which will take up some CSS-defined with in addition to the regular content width (usually 100% of the remaining width of the content)

In this scenario, provided the max content width of the theme is 720px, the markup for the three images could be something like:

<figure class="wp-block-image">
  <img src="..." srcset="..."
    sizes="(min-width: 720px) 720px, 100vw">
</figure>
<figure class="wp-block-image alignwide">
  <img src="..." srcset="..."
    sizes="(min-width: 720) calc(50% + 720px), 100vw">
</figure>
<figure class="wp-block-image alignfull">
  <img src="..." srcset="..."
    sizes="100vw">
</figure>

I say "could be" because the exact configuration here will change depending on the theme and any theme may have multiple different configurations depending on what template and other conditions are currently in play.

Current Behavior

At present there is no way of allowing this fine level of control via wp_calculate_image_sizes. While we can control the sizes attribute based on template and other configurations, the same sizes attribute will be applied to all images regardless of alignment. This is because we have no way of knowing the alignment attribute of individual images when wp_calculate_image_sizes runs.

In the current scenario, because the developer doesn't know what images are being displayed alignwide or alignfull, they have to assume all images are displayed full:

<figure class="wp-block-image">
  <img src="..." srcset="..."
    sizes="100vw">
</figure>
<figure class="wp-block-image alignwide">
  <img src="..." srcset="..."
    sizes="100vw">
</figure>
<figure class="wp-block-image alignfull">
  <img src="..." srcset="..."
    sizes="100vw">
</figure>

This invalidates the purpose of responsive images and stands in the way of performance optimization.

Possible Solution

A function / filter / something else is needed to be able to serve sizes attributes to individual images in a view depending on their current alignment. Since alignment is controlled using a class, it seems logical to provide a way of uncovering the current applied class to an displayed image so theme developers can build conditional statements around them.

Related Issues and/or PRs

It has been suggested that #869 is related, but from my reading it appears to tackle a different issue.

@davidwolfpaw
Copy link

This might be unrelated, but a method for handling this would be useful in areas where images are aligned wide or full to the content area, but not necessarily the viewport. For instance, if a theme were to have both full-width templates and content/sidebar templates.

I have to separately handle those templates (or am missing a better way), but being able to calculate based on content width in those varied cases would be useful in serving up the proper image sizes.

@mor10
Copy link
Contributor Author

mor10 commented Apr 12, 2018

@davidlaietta 100% related: in fact it is the exact same issue. My example just shows a full-width display.

@ellatrix ellatrix added this to the WordPress 5.0 milestone Apr 12, 2018
@ellatrix ellatrix added the [Type] Bug An existing feature does not function as intended label Apr 12, 2018
@mor10
Copy link
Contributor Author

mor10 commented Apr 12, 2018

@mor10
Copy link
Contributor Author

mor10 commented Apr 12, 2018

Some more thinking on this: The most obvious solution (from my perspective at least) is to add a $classes parameter to the wp_calculate_image_sizes hook holding the classes applied to the current <figure> element wrapping the image. The classes can then be used as conditions for different sizes configurations based on current display width.

@mor10
Copy link
Contributor Author

mor10 commented Apr 12, 2018

A suggestion of how this could work from a theme standpoint:

This assumes the <figure> element holding an image has a data-display-alignment attribute to match the alignment class applied attached that is somehow surfaced as a parameter in the wp_calculate_image_sizes hook:

/**
 * Add custom image sizes attribute to enhance responsive image functionality
 * for content images.
 *
 * @param string $sizes A source size value for use in a 'sizes' attribute.
 * @param array  $size  Image size. Accepts an array of width and height
 *                                      values in pixels (in that order).
 * @param string $data-display-alignment The alignment of the image.
 * @return string A source size value for use in a content image 'sizes' attribute.
 */
function my_content_image_sizes_attr( $sizes, $size, $alignment ) {
	$width = $size[0];

	// If image is as wide as main content, nothing special is needed.
	if ( 720 <= $width ) {
		$sizes = '100vw';
	} else {

		// Images displayed inside the regular content area.
		$sizes = '(min-width: 720px) 720px, 100vw';

		// Wide images: 720+50% of the available extra space.
		if ( 'wide' === $alignment ) ) {
			$sizes = '(min-width: 720) calc(50% + 720px), 100vw';
		}

		// Full images: 100% of the available extra space.
		if ( 'wide' === $alignment ) ) {
			$sizes = '100vw';
		}

		// If there's a sidebar kicking in at 720px and taking up 25% of the available width.
		if ( is_active_sidebar( 'sidebar-1' ) ) {
			
			// Images displayed inside the regular content area.
			$sizes = '(min-width: 720px) 720px, 100vw';
			
			// Wide images
			if ( 'wide' === $alignment ) ) {
				$sizes = '(min-width: 960px) calc(50% + 720px), (min-width: 720px) 75vw, 100vw';
			}

			// Full images
			if ( 'wide' === $alignment ) ) {
				$sizes = '(min-width: 720px) 75vw, 100vw';
			}
		}
	}

	return $sizes;
}
add_filter( 'wp_calculate_image_sizes', 'my_content_image_sizes_attr', 10, 3 );

@davisshaver
Copy link
Contributor

This is a good idea but it adds a layer of conceptual overhead not currently present. Is there any way this could be a later enhancement? I may be misinterpretating but I think this is already an issue that's just made more visible by Gutenberg.

@mor10
Copy link
Contributor Author

mor10 commented Apr 20, 2018

@davisshaver The current version of WordPress handles this in a sub-optimal yet acceptable way that works because the theme developer knows the exact max width of any displayed image. This can then be codified into how images added to the content is handled. The key is all images added to the content are handled the same way with the same max width.

With Gutenberg, the publisher can choose different image widths that span beyond the content width. The developer has no way of finding out what width any particular image is displayed in and because they can only treat all images within the content as the same width, they have to treat every image as if it's full width, something that most likely is only true in a very few instances.

The result is responsive images that don't do what they're supposed to, which in turn leads to significant page size increase.

It's a bug, and it must be fixed before release to avoid significant performance hits on every WordPress page with images.

@azaozz
Copy link
Contributor

azaozz commented Jun 12, 2018

Not sure if we will add $alignment to the 'wp_calculate_image_sizes' filter but we can pass the whole <img> tag, and even have some data-wp-* attributes added from the editor to give more context to the theme. There is already data-wp-percent-width, see #6177. Another one, of course, can be data-wp-alignment :)

@mor10
Copy link
Contributor Author

mor10 commented Jun 25, 2018

Did some more thinking around this in #6177: #6177 (comment)

@mtias mtias added the Customization Issues related to Phase 2: Customization efforts label Jul 19, 2018
@andreiglingeanu
Copy link
Contributor

More thoughts on that: #6177 (comment)

@mtias
Copy link
Member

mtias commented Nov 20, 2018

Progress in #11973, which ran into some implementation issues.

@kjetisve
Copy link

kjetisve commented Feb 28, 2019

Would you consider adding an option in the Cover Image to change the focal point as this plugin does: https://nb.wordpress.org/plugins/wp-smartcrop/
I see this is already an issue: #10925

@adamziel
Copy link
Contributor

adamziel commented Apr 22, 2020

It seems like this one is stale, I removed the Status: In Progress label.

@paaljoachim
Copy link
Contributor

Is this issue still valid?
It would be great with an update.
Thanks!

@skorasaurus skorasaurus added the [Feature] Media Anything that impacts the experience of managing media label Jan 22, 2021
@mtias mtias added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. and removed [Type] Bug An existing feature does not function as intended labels Mar 3, 2021
@mtias mtias mentioned this issue Mar 3, 2021
12 tasks
@youknowriad
Copy link
Contributor

This is also something that is now possible after #29335. Theme's can define a default "layout" that can be used. It will be available to all themes once the experimental-theme.json file is made stable. (hopefully in WP 5.8)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customization Issues related to Phase 2: Customization efforts [Feature] Media Anything that impacts the experience of managing media [Feature] Themes Questions or issues with incorporating or styling blocks in a theme.
Projects
None yet
Development

No branches or pull requests