Make WordPress Core

Opened 10 months ago

Last modified 8 months ago

#58884 accepted enhancement

The image size for the Site Logo block is hard coded to "full"

Reported by: asafm7's profile asafm7 Owned by: audrasjb's profile audrasjb
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch 2nd-opinion close
Focuses: Cc:

Description

The recommended size for the site icon is 512x512.

The Site Logo block is coded to use the "full" size, in general-templates.php:

$image = wp_get_attachment_image( $custom_logo_id, 'full', false, $custom_logo_attr );

If the icon is at the recommended size of 512x512, the full size is much too big for the common use of the Site Logo block (in the header). That leads to a "Properly size images" warning in page-speed tools.

Attachments (1)

58884.patch (713 bytes) - added by pitamdey 9 months ago.
After applying this solution the issue is resolved

Download all attachments as: .zip

Change History (16)

#1 @audrasjb
10 months ago

  • Milestone changed from Awaiting Review to 6.4

#2 @rajinsharwar
10 months ago

I would propose it to be set to medium instead of full.

#3 @Presskopp
10 months ago

why not
$image = wp_get_attachment_image( $custom_logo_id, array( 512, 512 ), false, $custom_logo_attr );
?

@pitamdey
9 months ago

After applying this solution the issue is resolved

#4 @audrasjb
9 months ago

  • Keywords has-patch commit added

The proposed solution looks good to me.
Self assigning for commit consideration.

#5 @Ankit K Gupta
9 months ago

Thank you, @asafm7 for the suggestion and @pitamdey for the patch.


Test Report. ✅

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58884/58884.patch

Env

  • WordPress - 6.4-alpha-20230824.164623
  • Web Server: Nginx
  • Chrome Version - 115
  • OS - macOS
  • Theme: Twenty Sixteen Version: 3.0
  • PHP - 7.4.0
  • Active Plugin - None


Test result

Site Logo block to be functioning as expected. I couldn't find any functional issues during testing.

Status

Ready for the Next Step. ✅

#6 follow-up: @audrasjb
9 months ago

  • Keywords commit removed
  • Owner set to audrasjb
  • Status changed from new to accepted

The implementation looks good to me, but could you please add some sources about why the ideal size would be 512x512px?

This ticket was mentioned in Slack in #core by zunaid321. View the logs.


9 months ago

#8 @asafm7
9 months ago

Maybe I'm missing something, but isn't the patch still hard coding 512*512, and that is usually too big for a logo?

#9 in reply to: ↑ 6 @dhrumilk
9 months ago

Replying to audrasjb:

The implementation looks good to me, but could you please add some sources about why the ideal size would be 512x512px?

  1. Compatibility: A 512x512 pixel size for the site icon is often recommended because it provides a good balance between high resolution and compatibility with various devices and screen resolutions. It ensures that the icon looks crisp and clear on both desktop and mobile devices without being excessively large.
  1. Retina Displays: Many modern devices, especially Apple devices, have high-resolution Retina displays. A 512x512 pixel site icon ensures that the icon still appears sharp and detailed on these high-resolution screens without being pixelated or blurry.
  1. Favicon Use: Site icons are often used as favicons, which are small icons displayed in the browser tab when a user visits a website. A 512x512 pixel icon can be easily scaled down to smaller sizes for use as favicons while maintaining image quality.
  1. Versatility: A 512x512 pixel icon provides versatility. It can be downsized as needed for different use cases within the website, such as in the header, footer, or as a favicon, without losing image quality.
  1. SEO and Accessibility: Having a high-quality site icon can also contribute to a better user experience and potentially improve search engine optimization (SEO) by making your website appear more professional and visually appealing.

#10 @asafm7
9 months ago

@dhrumilk the reason for opening the issue was that 512x512 is too big for a header logo.

On 4 you wrote "It can be downsized as needed for different use cases within the website, such as in the header", but as the patch keeps the size hard-coded, downsizing isn't possible. This is what the ticket was aimed to change. The patch doesn't address it.

The patch only replaces "full" with "512x512". When I opened the ticket, I assumed "full" to be "512x512" anyway, as it is the recommended size by WordPress for the icon image.

Am I missing something?

Last edited 9 months ago by asafm7 (previous) (diff)

#11 @oglekler
8 months ago

  • Keywords 2nd-opinion added

I wonder if there is an issue at all, because this image has srcset and browser picks optimal size by its own. And 512x512 size needs to be registered, otherwise it will have no effect, and if it is registered, then we will create all other images in this size.

Last edited 8 months ago by oglekler (previous) (diff)

#12 @mukesh27
8 months ago

  • Focuses performance removed
  • Keywords close added

@asafm7, the site icon and site logo are different, and you can use them at your preferred size.

Regarding the site logo, WordPress does not suggest a specific size because logos can vary in size according to the brand. Using the full size is a suitable choice in my opinion, as it provides users with the flexibility to select the size they want.

It's worth noting that there is a get_custom_logo_image_attributes filter available, which allows you to use different-sized images if needed.

Let's also take into consideration @oglekler's point mentioned above. Removing the Performance focus.

@audrasjb Is this in your radar for 6.4?

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

#14 @asafm7
8 months ago

Thanks @mukesh27 .

This option ties the logo to the icon.

https://drive.google.com/file/d/1_JG1EgS3jCU6fSl3jBzLLTLVLcXDLXOS/view?usp=drivesdk

I didn't try, but I believe filtering the attributes might be too late for utilizing tools such as Jetpack Photon, which contracts its URLs in an earlier stage.

#15 @audrasjb
8 months ago

  • Milestone changed from 6.4 to Awaiting Review

I agree with comments 11 & 12, I'm unsure about the value of this change.
Let's remove this from milestone for now, but keep it open for discussion.

Note: See TracTickets for help on using tickets.