Fix default featured image size by derweili · Pull Request #15844 · WordPress/gutenberg

derweili · 2019-05-27T10:21:14Z

Description

Fixed the default image sizes used for featured images displayed in the editor:
Fixes: #15842, #14084

How has this been tested?

Verified with a linter on my local environment.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

jorgefilipecosta · 2019-05-27T21:31:46Z

packages/editor/src/components/post-featured-image/index.js

 	let mediaWidth, mediaHeight, mediaSourceUrl;
 	if ( media ) {
-		const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId );
+		const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'thumbnail', media.id, currentPostId );


Hi @derweili, thank you for your contribution 👍
I verified that the classic editor also loads post thumbnail when it is available.
E.g: this is the markup classic editor provided (the src is for the post-thumbnail size):

I verified that if post_thumbnail is not available thumbnail size used in the classic editor while in Gutenberg full size is used. I think maybe we should follow the same logic verify if post_thumbnail is available and if yes we should use it, if not Gutenberg verifies if thumbnail is available.

Makes sense to me. The post-thumbnail image size is available if the theme adds support for post thumbnails.

thumbnail on the other hand is always there, but is usually something like 150x150 big.

@swissspidy thumbnail is not always there.

If the uploaded image is smaller than thumbnail size (defaut 150 x 150) the thumbnail size is missing.
full is the only size that's always available.

I think that the image sizes should be chosen in the following order:

Load post-thumbnail (still use the editor.PostFeaturedImage.imageSize filter)

If post-thumbnail is not available, load thumbnail

If both post-thumbnail and thumbnail are not available, use the full size.

Hi @derweili, that seems a good plan 👍

@jorgefilipecosta I implemented the changes as described above.

I reset the mediaSize to post-thumbnail and added a new fallbackMediaSize which has the value thumbnail by default.

I also added a new filter editor.PostFeaturedImage.fallbackImageSize to modify the new fallbackMediaSize, similar to the existing filter editor.PostFeaturedImage.imageSize

full is the only size that's always available.

The frustrating thing here is that media.media_details.sizes is an empty object when the image is smaller than the thumbnail size. If it always contained a size then we could do something to pick the appropriate size in the order of preference.

Given the current structure of the media.media_details object, this change looks good to me.

Alternatively, we could make another change so sizes always contains full at a minimum.

Any thoughts?

Thank you for the improvements @derweili this PR seems to be on a good path I left a suggestion.

Alternatively, we could make another change so sizes always contains full at a minimum.

It would be good if originally that was the case but now it may be risky to change the structure as there may be external code/filters that is expecting and empty array.
I think the current direction addresses the problem well.

mediaSize reset to post-thumbnail fallbackMediaSize added and set to thumbnail `editor.PostFeaturedImage.fallbackImageSize` filter added to modify the new fallbackMediaSize

jorgefilipecosta · 2019-09-19T17:24:37Z

packages/editor/src/components/post-featured-image/index.js

 	let mediaWidth, mediaHeight, mediaSourceUrl;
 	if ( media ) {
 		const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId );
+		const fallbackMediaSize = applyFilters( 'editor.PostFeaturedImage.fallbackImageSize', 'thumbnail', media.id, currentPostId );


Do we really need a new filter for the fallback? Can't we use the existing filter e.g:

const fallbackMediaSize = applyFilters( 'editor.PostFeaturedImage. imageSize', 'thumbnail', media.id, currentPostId );

Another thing to consider is that the filters used here may have complex logic it would be nice to only compute the fallbackMediaSize if the mediaSize does not exist.

@jorgefilipecosta

I implemented you suggestions in the latest commit.

The fallback size now uses the same filter as the default size.

The fallback size now get computed only when default size is missing

jorgefilipecosta · 2019-09-19T17:27:36Z

packages/editor/src/components/post-featured-image/index.js

 	let mediaWidth, mediaHeight, mediaSourceUrl;
 	if ( media ) {
-		const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId );
+		const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'thumbnail', media.id, currentPostId );


Thank you for the improvements @derweili this PR seems to be on a good path I left a suggestion.

Alternatively, we could make another change so sizes always contains full at a minimum.

It would be good if originally that was the case but now it may be risky to change the structure as there may be external code/filters that is expecting and empty array.
I think the current direction addresses the problem well.

jorgefilipecosta

Thank you for all the interactions @derweili I left a suggestion but other than that the PR seems to be very close to the finish line.

improved: the fallback size will now be calculated only if the default size is not available. changed: the fallback size now uses the same filter as the default size

jorgefilipecosta

LGTM 👍

…rnmobile/gb-mobile-872-JSApplicationIllegalArgumentException-in-RCTAztecView * 'master' of https://github.com/WordPress/gutenberg: (56 commits) Update: Default gradients. (#18214) Fix: setting a preset color on pullquote default style makes the block invalid (#18194) Fix default featured image size (#15844) Fix postmeta radio regression. (#18183) Components: Switch screen-reader-text to VisuallyHidden (#18165) [rnmobile] Release 1.16.0 to master (#18261) Template Loader: Add theme block template resolution. (#18247) Add a README file for storybook directory (#18245) Add editor-gradient-presets to get_theme_support (#17841) Add "Image Title Attribute" as an editable attribute on the image block (#11070) enables horizontal movers in social blocks (#18234) [RNMobile] Add mobile Spacer component (#17896) Add experimental `ResponsiveBlockControl` component (#16790) Fix mover for floats. (#18230) Rename Component to WPComponent in docstring (#18226) Colors Selector: replace `Aa` text by SVG icon (#18222) Removed gif from README (#18200) makes the submenu items stacked vertically (#18221) Add block navigator to sidebar panel for nav block (#18202) Fix: consecutive updates may trigger a blocks reset (#18219) ...

Change featured image default size to thumnail

7c1c541

derweili requested review from talldan and youknowriad as code owners May 27, 2019 10:21

youknowriad requested a review from a team May 27, 2019 15:45

youknowriad added Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Type] Bug An existing feature does not function as intended [Feature] Media Anything that impacts the experience of managing media labels May 27, 2019

jorgefilipecosta reviewed May 27, 2019

View reviewed changes

gziolo added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jun 3, 2019

fallbackMediaSize added to featured image

01dad3a

mediaSize reset to post-thumbnail fallbackMediaSize added and set to thumbnail `editor.PostFeaturedImage.fallbackImageSize` filter added to modify the new fallbackMediaSize

jorgefilipecosta mentioned this pull request Jul 15, 2019

Fix: change featured image size to medium #15842 #15963

Closed

jorgefilipecosta reviewed Sep 19, 2019

View reviewed changes

PostFeaturedImage Performance optimizations

d1d2547

improved: the fallback size will now be calculated only if the default size is not available. changed: the fallback size now uses the same filter as the default size

derweili requested a review from jorgefilipecosta October 29, 2019 15:59

jorgefilipecosta approved these changes Nov 4, 2019

View reviewed changes

jorgefilipecosta merged commit 61753df into WordPress:master Nov 4, 2019

jorgefilipecosta mentioned this pull request Nov 4, 2019

Post thumbnail loading full size #14084

Closed

derweili deleted the featured-image-size branch November 8, 2019 13:00

youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix default featured image size#15844

Fix default featured image size#15844
jorgefilipecosta merged 3 commits intoWordPress:masterfrom
derweili:featured-image-size

derweili commented May 27, 2019 •

edited

Loading

Uh oh!

jorgefilipecosta May 27, 2019

Uh oh!

swissspidy May 28, 2019

Uh oh!

derweili May 29, 2019

Uh oh!

jorgefilipecosta Jul 4, 2019

Uh oh!

derweili Jul 9, 2019 •

edited

Loading

Uh oh!

pablinos Aug 18, 2019

Uh oh!

jorgefilipecosta Sep 19, 2019

Uh oh!

jorgefilipecosta Sep 19, 2019

Uh oh!

derweili Oct 28, 2019

Uh oh!

jorgefilipecosta Sep 19, 2019

Uh oh!

jorgefilipecosta left a comment

Uh oh!

jorgefilipecosta left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

derweili commented May 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derweili Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

derweili commented May 27, 2019 •

edited

Loading

derweili Jul 9, 2019 •

edited

Loading