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

Click-to-expand images could prefetch full-resolution image to speed up display #60883

Open
westonruter opened this issue Apr 18, 2024 · 4 comments
Labels
[Block] Image Affects the Image Block [Type] Enhancement A suggestion for improvement.

Comments

@westonruter
Copy link
Member

What problem does this address?

I just noticed that hovering over a click-to-expand image block does not proactively attempt to prefetch the full-scale image. This can result in a prolonged period in which the low-resolution scaled-up image is displayed in the lightbox:

Screen.recording.2024-04-18.15.57.10.webm

This seems like a missed opportunity.

What is your proposed solution?

As has been explored in #59707 for client-side navigations wherein hovering over a link will prefetch it for faster rendering (cf. discussion about speculation rules), I suggest implementing the same for prefetching the full-resolution image.

Right after this line:

$p->set_attribute( 'data-wp-on--click', 'actions.showLightbox' );

I suggest adding a pointerenter event directive (and perhaps a pointerdown too for earlier detection on touch screens):

$p->set_attribute( 'data-wp-on--pointerenter', 'actions.prefetchImage' ); 
$p->set_attribute( 'data-wp-on--pointerdown', 'actions.prefetchImage' ); 

The prefetchImage action should be debounced with a 200 millisecond timeout (to match the moderate eagerness in speculation rules), after which the high-resolution image URL would be prefetched.

I'd like to work on this myself if this is deemed worthwhile.

@artemiomorales
Copy link
Contributor

artemiomorales commented May 3, 2024

Hey @westonruter, thanks for creating this issue! We were previously loading the image on hover but it was removed due to concerns that it would cause unnecessary loading of many images, particularly on large pages. Here's a comment where it was mentioned, and the functionality was removed in this commit.

I see that there's been more discussion around matching speculation rules since then. Right now we're taking the conservative eagerness approach in the link you mentioned. I'm fine with either approach but will ping @SantosGuillamot @cbravobernal @jasmussen for thoughts.

Quick summary of the discussion: We previously had prefetching on hover but removed it, and since then there's been discussion on potentially adding it back, along with an open PR.

@westonruter
Copy link
Member Author

It seems that code was preloading the image immediately on hover? If there is a 200 ms timeout to start preloading would that have prevented the issue?

@SantosGuillamot
Copy link
Contributor

I'm fine with any option. Preloading the image on hover if there is a timeout seems fine to me. Back then, it was downloading most of the images while scrolling down, and I guess this would avoid that problem.

Having said that, should we have a setting/filter in case users want to change the preloading strategy?

@jasmussen
Copy link
Contributor

I'd tend to support the goal of prefetching on hover, it can substantially improve the feature. The fact that we show a zoom cursor suggests you're just zooming the image, not loading it. I recognize there's all sorts fof technical challenges to making this happen, but conceptually it would help match expectations. I'd avoid a user setting that featuers a UI, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants