3.16 enhancement - Images intersecting the viewport should be considered as LCP candidates about wp-rocket HOT 9 OPEN

About keeping intersecting images:

The issue comes from the beacon script in the LCPCandidates: https://github.com/wp-media/wp-rocket/blob/b459f191a348426b2b1a3cf656a4db577e76800f/assets/js/lcp-beacon.js#L9C1-L16C12
This condition is checking that the image is fully in the viewport. Instead, we should check that it is intersecting only. We have such a check already for the ATF images here.

• We should make a dedicated function isIntersecting(rect) returning a boolean based on the check already existing, and use this in both here and here.

About accounting only for the visible area for LCP "score"

The area to account for is computed here. It is currently accounting for the complete image size.
To account for the visible part only, we should do:

const visibleWidth = Math.min(rect.width, (window.innerWidth || document.documentElement.clientWidth) - rect.left));
const visibleHeight = Math.min(rect.height, (window.innerHeight || document.documentElement.clientHeight) - rect.top))
const area = visibleWidth * visibleHeight;

For the testing part, TBC with @jeawhanlee but it might be more suited to test this with Rocket-E2E.

Effort: [XS] (test scenarios are outside the scope as they are already on Rocket-E2E, if we will use Rocket-E2E).

from wp-rocket.

Should this be moved to Ready for Review?

from wp-rocket.

@Miraeld @wp-media/qa-team Do we have test scenarios, maybe on Rocket-E2E to validate this? Maybe similar to the acceptance criteria?

from wp-rocket.

@MathieuLamiot not from what I am aware of.

from wp-rocket.

@MathieuLamiot , @Miraeld just pushed these 2 templates on /wp-media/rocket-test-data:
lcp_images_intersecting_viewport_A.php
lcp_images_intersecting_viewport_B.php
that should cover the AC.
Will provide you with pages created on e2e as well, in a moment.

from wp-rocket.

@MathieuLamiot , @Miraeld
on e2e, scenario 1

ImageA (size: 200x200, fully in the viewport) and ImageB (size:300x300, only 300x1 visible in the viewport)

we have page link)

and for scenario 2

ImageA (size: 200x200, fully in the viewport) and ImageB (size:300x300, only 300x250 visible in the viewport)

we have page link

TC link.

from wp-rocket.

@Miraeld Have you performed basic checks to ensure the code does not break on happy path and that the reported failing cases are fixed? In which case, we can merge so that we only have to work on the feature branch. OK? Thanks 🙏

from wp-rocket.

Kind reminder @Miraeld: can this be merged?

from wp-rocket.

I did basic check yea, and it worked.
I think it can be merged yes.

from wp-rocket.