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

Image classes missing in render #11603

Closed
strarsis opened this issue Nov 7, 2018 · 16 comments
Closed

Image classes missing in render #11603

strarsis opened this issue Nov 7, 2018 · 16 comments
Assignees
Labels
[Block] Image Affects the Image Block [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@strarsis
Copy link
Contributor

strarsis commented Nov 7, 2018

Steps to reproduce the behavior:

  1. Add image block to page.
  2. Select an example for image for this image block.
  3. Select a specific image size.
  4. Save the page
  5. Open the page in frontend, notice that neither the <img element nor its wrapper elements have an image size related classes assigned. This is very useful for themes, especially when the theme register its own image sizes that are more special purpose.

Expected behavior
That the either the resulting <img element or its wrapper elements has the image-size related classes assigned.

Desktop (please complete the following information):

  • OS: Windows 10 Pro x64
  • Browser Chrome
  • Version Latest Chrome

Additional context
WordPress 4.9.8
Gutenberg 4.2.0

@strarsis strarsis changed the title Image class missing in render Image classes missing in render Nov 7, 2018
@designsimply designsimply added [Type] Enhancement A suggestion for improvement. [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media [Block] Image Affects the Image Block labels Nov 11, 2018
@designsimply
Copy link
Member

Adding an example of the current HTML from an image block compared to the HTML for the same image in the classic editor for reference:

<figure class="wp-block-image"><img src="http://alittletestblog.com/wp-content/uploads/2018/11/IMG_20180304_000528-150x150.jpg" alt="" class="wp-image-15568"/></figure>

Tested with WordPress 4.9.8 and Gutenberg 4.3.0-rc.1 using a thumbnail-sized image.

<img class="alignnone size-thumbnail wp-image-15568" src="http://alittletestblog.com/wp-content/uploads/2018/11/IMG_20180304_000528-150x150.jpg" alt="" width="150" height="150" />

Tested with WordPress 4.9.8 using a thumbnail-sized image.

Aside: I noticed a semi-related PR wants to add an is-resized class to images using a custom height and width—it is not the same but related because it add a class about image size: #9421

@strarsis thank you for asking about this! If you are able to add an example for the situation you mentioned where a specific size that has a special purpose and how the theme would use the classes such as size-thumbnail, size-medium, size-large, size-full, it may help document better why this change is wanted.

@strarsis
Copy link
Contributor Author

strarsis commented Nov 11, 2018

@designsimply: Let's have for example a 'portrait' image size. The theme can now make it rounded using border-radius. Or some extra decorative elements using pseudo-elements. Or some CSS based filter, or even using JavaScript for effects.
I found this kind of hook quite useful in the past in several themes and adding extra classes would be redundant and much more complicated for the user because usually one would have to enter them manually.

@gschoppe
Copy link

gschoppe commented Dec 5, 2018

Why is this tagged as a feature request and an enhancement?? this is a breaking reversion from how images have worked in WordPress for several years. We need parity with the existing classes for theme compatibility.

@bupotalovo
Copy link

@gschoppe used logic reasoning. It's not very effective...

@designsimply designsimply added Needs Design Feedback Needs general design feedback. Needs Decision Needs a decision to be actionable or relevant and removed [Type] Enhancement A suggestion for improvement. labels Dec 5, 2018
@designsimply
Copy link
Member

designsimply commented Dec 5, 2018

@gschoppe @bupotalovo sometimes I get a label wrong! A helpful explanation for why it should be changed is always much appreciated!! My original thinking for this case was that the new syntax is intentional and so changing it would be an update to something existing and not something that's broken—however!, I understand that it can be a fine line depending on the case/context and it's very possible for me to get it wrong sometimes.

I've swapped out [Type] Enhancement for Needs Design Feedback and Needs Decision to get extra help for this case.

@gschoppe
Copy link

gschoppe commented Dec 5, 2018

Thanks for the reclassification.

This may be an intentional decision, but it does remove the ability for front-end scripts and stylesheets to reason about the thumbnail size being used. For an edge case example, my wp-smartcrop plugin needs to apply a custom class to any image in the content that has a specific metakey assigned to the ID and is not a cropped size. The ID part is functional, but I cannot check the image's size class to identify whether we are using a cropped size or not.

Another really simple use case would be adding border radius to all images that have the size-profile class, or filtering which responsive image sizes are rendered and which are not, based on the current image's crop.

@bupotalovo
Copy link

@designsimply Thanks for clarification. The Label [Type] Bug is still missing though.

@gschoppe
Copy link

gschoppe commented Dec 14, 2018

@mtias can we get a little attention to this issue? At the moment, thumbnail-size and image dimensions are not made available to PHP or front-end JS by Gutenberg at all. So filters can't modify the functionality of the image block to provide progressive enhancements.

Examples of things that are currently not possible with the lack of data provided to PHP/JS:

  • defining an area with the proper aspect ratio for an image to fill, before it loads, thereby preventing unsightly document reflow
  • inlining a 3x3px version of the image, then lazy-loading the actual image, as seen on Medium (we don't know if the image is a cropped thumb size or not, so we can't choose the correct micro-thumb to embed).
  • styling images differently, based on which thumbnail size was chosen (a choice that clearly carries context with it)
  • lazy loading different image sizes by screen size (without knowing the thumb name, we don't know whether it is a cropped thumb or not, or which crop to use, so we don't know how to match the crop area with the size we lazy-load)
  • replacing image urls if the attachment post is updated. (since attachments are a post type, that post should be the single source of truth about the image. without knowing thumb-size we cannot dynamically replace the image if the underlying attachment changes)
  • Using modern image formats when appropriate. We cannot live replace the images with webp or other modern formats without knowing which thumbnail size is needed.

Even if this data was just stored in the attributes of the block and not rendered, we could use the render filter to enhance this functionality where needed, but as it is, the context is just lost entirely.

@danfoy
Copy link

danfoy commented Feb 27, 2019

I'm also struggling with this. Removing the size class attributes makes it impossible to reason about the images included in the document for mobile-first design... I have to treat a 150px thumbnail the same as a 1200px centrepiece.

This is a real shame because I really like the Gutenberg interface, but converting classic editor posts to Gutenberg breaks themes. Out of all the class spam within WordPress, I really don't understand why this semantic and genuinely useful information is what's been chosen to be thrown away.

@gschoppe
Copy link

The most frustrating thing about this is that we can't even patch it in with a block filter, since the block doesn't even know what size was selected in its attributes blob, it just has an ID and a URI, which is not sufficient data to describe the state of the media library.

@mapk
Copy link
Contributor

mapk commented Apr 5, 2019

This seems pretty important. I'd like to suggest we move forward with it. Shouldn't be too difficult to append a className to the image when rendered.

Let's get some dev help on this. 👍

@mapk mapk added Needs Dev Ready for, and needs developer efforts and removed Needs Decision Needs a decision to be actionable or relevant Needs Design Feedback Needs general design feedback. labels Apr 5, 2019
@mapk mapk added this to the 5.5 (Gutenberg) milestone Apr 5, 2019
@draganescu draganescu self-assigned this May 2, 2019
@draganescu
Copy link
Contributor

I am working on this.

@gziolo gziolo removed this from the 5.6 (Gutenberg) milestone May 6, 2019
@draganescu
Copy link
Contributor

Hi there is a draft PR #15464 which solves this issue, however it still needs a bit of refactoring :)

@noisysocks noisysocks added the [Type] Bug An existing feature does not function as intended label Jul 10, 2019
@noisysocks
Copy link
Member

#15464 fixed this.

@gschoppe
Copy link

gschoppe commented Jul 10, 2019

Is there any reason why this class can't be duplicated on the actual img tag? Doing so would provide back compatibility with existing code that relies on these size classes. As it is, the functionality exists, but will require a rewrite of any existing code targeting them.

@draganescu
Copy link
Contributor

I don't think there is any limitation not to do it. The way it was implemented was simply because I followed along the current pattern of using the container figure for styling. You could open a separate issue to see if anyone else has valuable input on duplicating or not :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

9 participants