gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Add Image Prefetching for Click to expand Images

Open Takshil-Kunadia opened this issue 1 year ago • 3 comments

PR for #60883

What?

Prefetch full-resolution image to speed up display of Click to expand images. see https://github.com/WordPress/gutenberg/issues/60883 for more details

Why?

Hovering over a click-to-expand image block does not proactively attempt to prefetch the full-scale image in Lighbox. This can result in a prolonged period in which the low-resolution scaled-up image is displayed in the lightbox.

How?

  • Implement prefetchImage action that prefetches an image and keeps it in the browser cache.
  • Then utilising the above add pointer event directives that trigger it on pointerup & pointerdown.

Testing Instructions

  1. Create a post with a click to expand image on it.
  2. Open Dev tools and throttle network to slow 3G and notice the network tab.
  3. Hover over the image, this will trigger the image to load.
  4. When the loading is complete, click the image for it to load in full resolution without any delay or loading.

Testing Instructions for Keyboard

Screenshots or screencast

Screenshot 2024-04-25 at 10 18 51 PM

Takshil-Kunadia avatar Apr 25 '24 16:04 Takshil-Kunadia

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Takshil-Kunadia <[email protected]>
Co-authored-by: luisherranz <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: artemiomorales <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions[bot] avatar Apr 25 '24 16:04 github-actions[bot]

@Takshil-Kunadia Thanks for creating this PR! I've left a comment on the issue to loop a few other folks into the discussion who may have thoughts on how to best proceed. In short, we previously had prefetching on hover but removed it. Since then, there's been more discussion around this, so perhaps may be good to add this back.

artemiomorales avatar May 03 '24 16:05 artemiomorales

Thanks @artemiomorales ! for the context and for looping the stakeholders in the discussion! I've updated the code to reflect the direction discussed in the issue. Happy to adjust further based on any additional feedback 😄

Takshil-Kunadia avatar Jun 17 '25 18:06 Takshil-Kunadia

I'm just realizing a problem here with the overall image lightbox: what if the original src image is too large for the viewport when the image is expanded (e.g. on mobile)? Shouldn't the expanded lightbox image copy the srcset as well? If this is the case, prefetching the src wouldn't necessarily be correct since a smaller size could be more appropriate. This could be achieved simply by copying the srcset of the original image to lightbox image and also adding it to the imagesrcset of the prefetch link. In addition to this, an appropriate sizes attribute would need to be created specifically for the lightbox image, although the browser default of 100vw would probably be correct for the lightbox image. In this way, the lightbox would avoid downloading a larger image than needed, and this further speeding up displaying of the expanded image.

westonruter avatar Jun 21 '25 15:06 westonruter

Thanks! @westonruter for looking into this 🙇🏻‍♂️ and yes good catch! it makes sense to have responsive images for lightbox images to avoid unnecessarily large downloads, especially on smaller viewports.

One thing I’d like to highlight though is that imagesrcset and imagesizes are supported with preload, not with prefetch afaik (reference: https://web.dev/articles/preload-responsive-images). This would slightly deviate from the original approach discussed in the issue which was on using prefetch as part of speculative loading.

That said, I’ve implemented the suggested preload approach in https://github.com/WordPress/gutenberg/pull/61107/commits/d7f2e41da920ac2fe91707e64e3f77cc075b15a2

Takshil-Kunadia avatar Jun 22 '25 08:06 Takshil-Kunadia

Hi @westonruter I've implemented the refactor as per your last comment. Since the PR has been approved, just wanted to check if it’s good to merge ✅

Takshil-Kunadia avatar Jul 09 '25 07:07 Takshil-Kunadia

Hi @westonruter, just following up on this PR since it’s been approved. Please let me know if there's anything else needed from my side to get it merged. Thanks ☺️!

Takshil-Kunadia avatar Aug 04 '25 07:08 Takshil-Kunadia

@Takshil-Kunadia I'll defer to @luisherranz for final approval and merge.

westonruter avatar Aug 05 '25 00:08 westonruter

Rebased to keep this branch up-to-date with trunk and resolve conflicts ✅

Takshil-Kunadia avatar Oct 16 '25 08:10 Takshil-Kunadia

Hey @luisherranz This PR has already been reviewed and approved, so just wondering what’s next in the release process. Would love to see it land soon!

Takshil-Kunadia avatar Oct 30 '25 04:10 Takshil-Kunadia

Thanks @luisherranz implemented the changes 🙇

Takshil-Kunadia avatar Nov 03 '25 12:11 Takshil-Kunadia

Thanks for the approval @luisherranz! 🙇

Takshil-Kunadia avatar Nov 04 '25 09:11 Takshil-Kunadia