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

Add `wp-image-####` CSS class to images in the Media and Text block #11922

Merged
merged 3 commits into from Nov 16, 2018

Conversation

Projects
None yet
5 participants
@azaozz
Contributor

azaozz commented Nov 15, 2018

This is used to add srcset and sizes attributes on the front-end. As we use the "large" image size now, having these attributes becomes (almost) mandatory :)

Add `wp-image-####` class name to images
This is used to add `srcset` and `sizes` attributes on the front-end.

@azaozz azaozz added this to the WordPress 5.0 milestone Nov 15, 2018

@azaozz azaozz requested a review from youknowriad Nov 15, 2018

@youknowriad youknowriad modified the milestones: WordPress 5.0, 4.5 Nov 15, 2018

@mtias mtias requested a review from jorgefilipecosta Nov 15, 2018

@mtias mtias added the Media label Nov 15, 2018

} = attributes;
const mediaTypeRenders = {
image: () => <img src={ mediaUrl } alt={ mediaAlt } />,
image: () => <img src={ mediaUrl } alt={ mediaAlt } className={ ( mediaId && mediaType === 'image' ) ? `wp-image-${ mediaId }` : null } />,

This comment has been minimized.

@mtias

mtias Nov 15, 2018

Contributor

Do we need a deprecation just for the className?

This comment has been minimized.

@mtias

mtias Nov 15, 2018

Contributor

cc @aduth

This comment has been minimized.

@jorgefilipecosta

jorgefilipecosta Nov 16, 2018

Member

In this case given that the class being added is an internal one we need the deprecations.

@aduth

This comment has been minimized.

Member

aduth commented Nov 15, 2018

This is used to add srcset and sizes attributes on the front-end.

Can't this be determined via mediaId?

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 15, 2018

Can't this be determined via mediaId?

Yes, this is pretty much the same as in the image block. When there is a mediaId we add the wp-image-#### CSS class to the img tag so it's picked by wp_image_add_srcset_and_sizes() on display.

@jorgefilipecosta

I tested this changes and it looks like it works as expected, thank you for this enhancement @azaozz. On the frontend I'm now getting sizes and srcset on images added on media text block.
I added a commit that updates our text fixtures, I think this is ready.


};

This comment has been minimized.

@jorgefilipecosta

jorgefilipecosta Nov 16, 2018

Member

nitpick: I think we added an unintentional newline here.

This comment has been minimized.

@azaozz

azaozz Nov 16, 2018

Contributor

Uh, sorry.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 16, 2018

Thanks! Looks great. Was going to fix the tests tonight but got sidetracked a bit.

@azaozz azaozz merged commit 7e814e3 into master Nov 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@azaozz azaozz deleted the fix/media-text-add-image-class-for-srcset branch Nov 16, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Add `wp-image-####` CSS class to images in the Media and Text block (W…
…ordPress#11922)

* Add `wp-image-####` class name to images. Needed to add `srcset` and `sizes` attributes to the images on the front-end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment