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

Use the large image size when inserting from the media library. #6945

Closed
wants to merge 1 commit into from

Conversation

joemcgill
Copy link
Member

Description

For performance, avoid using the full size image by default when inserting an image. Note that the media object from the media library will include a url property for each image size, while media objects build returned by the REST API via the mediaUploader component will have a source_url property instead, so this needs to cover both cases.

See #6177.

How has this been tested?

Tested by adding and selecting an image via both the media uploader and the wp.media modal and checking to see that the large size was selected rather than the full size.

Types of changes

  • This is an enhancement to improve performance of the default image selection in Gutenberg for image blocks.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

For performance, avoid using the full size image by default when inserting an image. Note that the media object from the media library will include a `url` property for each image size, while media objects build returned by the REST API via the mediaUploader component will have a `source_url` property instead, so this needs to cover both cases.
@aduth
Copy link
Member

aduth commented May 24, 2018

IIRC, isn't the default meant to be a user preference saved between sessions based on the last explicitly set image size?

@joemcgill
Copy link
Member Author

There is a image_default_size option that we could use here, but currently even that is not implemented in Gutenberg, so this was a first pass at improving the current state. Even so, some blocks will require different default sizes, e.g. a gallery block vs a cover image.

@karmatosed karmatosed requested a review from a team May 31, 2018 20:34
@joemcgill
Copy link
Member Author

Looks like this needs a refresh since #6960 landed.

@ellatrix
Copy link
Member

ellatrix commented Jun 4, 2018

I believe @azaozz is working on a similar PR to add srcset to the images. Would be nice to have that in one go for all image blocks so there's no decrease in quality when it's needed. I believe it involves adding a srcset property to the image object in the media library. image_default_size should no longer be used as it's irrelevant. Also the image sizing options should be removed unless there are custom ones of different dimensions.

@danielbachhuber
Copy link
Member

image_default_size should no longer be used as it's irrelevant. Also the image sizing options should be removed there are custom ones of different dimensions.

@iseulde Can you point me to where I can learn more about this? Given each image size can have different crop proportions, I'm curious how srcset is meant to be a replacement.

@azaozz
Copy link
Contributor

azaozz commented Jun 5, 2018

IIRC, isn't the default meant to be a user preference saved between sessions...

Not any more :) There is no point in showing "image sizes" based on the file sizes created on uploading an image. If it is really needed, we can have some sizes defined in the media modal so the user can pick one, but they have been missing in Gutenberg from the beginning, and I think this is the right way to do this.

If the users want to resize the images, using the 25-50-75-100% buttons are a much better option. Another one is using the resize handles. However these buttons and handles will have to work on the visible dimensions of the images, not the (irrelevant) physical file size, i.e. in percentage of the actual visible image and not in pixels of the image file.

Given each image size can have different crop proportions, I'm curious how srcset is meant to be a replacement.

The srcset attribute replaces the src attribute when present, i.e. the browser ignores the src and loads one of the image files from srcset.

Generally in WP only the thumbnail sub-size is cropped. Some themes and plugins may add other cropped sub-sizes that are not proportional to the original image. All of these are excluded when generating the srcset (see wp_calculate_image_srcset()). If one of these "non-standard" sub-sizes is used in a post, and there aren't any other sub-sizes with the same proportions, the above function will not generate a srcset attribute for that image.

@ellatrix
Copy link
Member

ellatrix commented Jun 5, 2018

@iseulde Can you point me to where I can learn more about this? Given each image size can have different crop proportions, I'm curious how srcset is meant to be a replacement.

Updated the comment. The word "unless" was missing.

@aduth
Copy link
Member

aduth commented Sep 13, 2018

Is there still a relevancy / desire to continue the work here?

@aduth aduth added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 13, 2018
@aduth
Copy link
Member

aduth commented Oct 15, 2018

Closing as stale without response. It can be reopened if development is expected to continue, or a new pull request can be submitted.

@aduth aduth closed this Oct 15, 2018
@aduth aduth deleted the fix/image-selection-handling branch October 15, 2018 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants