Making WordPress.org

Opened 7 months ago

Closed 7 months ago

#7620 closed defect (bug) (fixed)

The issue of blurry images in the WordPress repository within plugins on the internal page.

Reported by: alexodiy's profile alexodiy Owned by: dufresnesteven's profile dufresnesteven
Milestone: Priority: normal
Component: Plugin Directory Keywords: needs-design-feedback has-screenshots
Cc:

Description

In the WordPress repository, plugins are allowed to upload banners with the following dimensions:

  • 772x250 px
  • 1544x500 px

However, in the WordPress repository, the design changes, and on the internal page of any plugin https://wordpress.org/plugins/call-to-action-block-wppool/ , we encounter the effect of blurry images (https://img.air-wp.com/2024-05-01_074250.png). This is due to the use of the 772x250px image.

Let's take an example with this plugin. In 2k resolution, the image used is sized 772x250px.

Let's look at the code:

The main container has a width of 1160px https://img.air-wp.com/2024-05-01_075316.png , previously this container was 920px wide, and even at that resolution, we experienced the effect of blurry images because the 772x250 image is being used instead of 1544x500.

#plugin-banner-call-to-action-block-wppool {
    background-image: url('https://ps.w.org/call-to-action-block-wppool/assets/banner-772x250.jpg?rev=2862263');
}

@media only screen and (-webkit-min-device-pixel-ratio: 1.5), only screen and (min-resolution: 120dpi) {
    #plugin-banner-call-to-action-block-wppool {
        background-image: url('https://ps.w.org/call-to-action-block-wppool/assets/banner-1544x500.jpg?rev=2862263');
    }
}

here https://img.air-wp.com/2024-05-01_075852.png

The problem here is that the media query specifies min-resolution: 120dpi, which corresponds to devices with a pixel density of around 1.5. For example, it works correctly on Macbooks.

Pixel density (DPI or PPI) indicates how many pixels fit into a certain area of the screen. This affects how sharp and detailed the images will appear on the screen.

In our case, if the device has a high pixel density (for example, in Retina displays), our condition triggers and everything is displayed correctly. But this does not work on regular classic displays. For example, if we open dev tools and enable a tool that allows us to change the screen resolution, the pixel density changes and the condition starts working, but regular users see a blurry image format.

And actually, there could be several solutions here:

  1. Disregard pixel density and consider pixel accuracy instead, for example:
    #plugin-banner-call-to-action-block-wppool {
        background-image: url('https://ps.w.org/call-to-action-block-wppool/assets/banner-772x250.jpg?rev=2862263');
    }
    
    @media only screen and (min-width: 933px) {
        #plugin-banner-call-to-action-block-wppool {
            background-image: url('https://ps.w.org/call-to-action-block-wppool/assets/banner-1544x500.jpg?rev=2862263');
        }
    }
    
  1. Keep the pixel density, but write additional media queries that take into account the screen resolution. However, in this case, it will clutter the CSS code, so it's easier to use the first solution.

Let's brainstorm together how to solve this problem.

Earlier, a similar issue was referenced in this ticket: https://meta.trac.wordpress.org/ticket/6617 by @dd32 and @jonoaldersonwp.

Attachments (1)

7620.diff (1.9 KB) - added by dufresnesteven 7 months ago.

Download all attachments as: .zip

Change History (12)

#1 @dufresnesteven
7 months ago

  • Owner set to dufresnesteven
  • Status changed from new to assigned

#2 follow-up: @dufresnesteven
7 months ago

Let's brainstorm together how to solve this problem.

Does the solution suggested here not work? (Apologies if I'm missing the point).

https://meta.trac.wordpress.org/ticket/6617

#3 in reply to: ↑ 2 @dd32
7 months ago

Replying to dufresnesteven:

Does the solution suggested here not work? (Apologies if I'm missing the point).

The issue is that currently the markup is using a DPI-target, such that only a high-dpi device is told to use the higher-resolution image.

Using srcset as proposed in #6617 will likely work, the approach taken here with width would be detrimental for high-dpi devices with smaller viewports AFAIK.

#4 @dd32
7 months ago

  • Component changed from General to Plugin Directory
  • Milestone Plugin Directory v3.0 deleted

#5 @dufresnesteven
7 months ago

I've attached a diff that renders:

<div class="plugin-banner" id="plugin-banner-{id}">
  <img 
   decoding="async" 
   fetchpriority="high" 
   src="/banner-772x250.png?rev=2903066" 
   srcset="/banner-772x250.png?rev=2903066 772w, /banner-1544x500.png?rev=2903066 1544w" 
   sizes="(min-width: 900px) 1544px, 772px">
</div>

It seems to work as we expect:

I haven't tested this on Edge but it appears like it should work. Thoughts?

@dufresnesteven
7 months ago

#6 @dufresnesteven
7 months ago

In 13670:

Plugin-directory: Update plugin banner html to better support display on lower DPI screens.

See #7620

#7 @dd32
7 months ago

[13671] missed the ticket

wporg-plugins-2024: Update plugin banner styles.

See: https://meta.trac.wordpress.org/ticket/7620
Props: alexodiy.

#8 @dufresnesteven
7 months ago

@alexodiy I've updated the code with some srcset magic. Do you mind testing and confirming that it's working as expected?

#9 @dd32
7 months ago

In 13673:

Plugin Directory: Add an empty alt tag to the banner.

See [13670].
See #7620.

#10 @dd32
7 months ago

#5515 was marked as a duplicate.

#11 @dufresnesteven
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

I've tested the fix in production using my developer tools and a VM. It appears to work.

If the issue persists, let me know.

Thanks!

Note: See TracTickets for help on using tickets.