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

Closes #6527: Images intersecting viewport considered as lcp candidates #6549

Conversation

Miraeld
Copy link
Contributor

@Miraeld Miraeld commented Apr 9, 2024

Description

Fixes #6527

This PR modifies the lcp-beacon.js file to change the way the Largest Contentful Paint (LCP) is calculated. The changes include creating a new function isIntersecting(rect) that checks if an image is intersecting the viewport and modifying the area computation to account only for the visible part of the image.

Documentation

User documentation

This change will improve the accuracy of the LCP calculation by considering images that intersect the viewport, not just those fully within it. This can lead to more accurate performance measurements and better user experience.

Technical documentation

The isIntersecting(rect) function checks if an image is intersecting the viewport by comparing the top and left properties of the image's bounding rectangle with the viewport's dimensions. The area computation is modified to calculate the visible area of the image by taking the minimum of the image's dimensions and the viewport's dimensions minus the image's position.

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

New dependencies

N/A

Risks

N/A

Checklists

Feature validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.

Documentation

  • I prepared the user documentation for the feature/enhancement and shared it in the PR or the GitHub issue.
  • I prepared the technical documentation if needed, and shared it in the PR or the GitHub issue.

Code style

  • I wrote self-explanatory code about what it does.
  • I wrote comments to explain why it does it.
  • I named variables and functions explicitly.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • I listed the introduced external dependencies explicitly on the PR.
  • I validated the repo-specific guidelines from CONTRIBUTING.md.

@Miraeld Miraeld added the type: enhancement Improvements that slightly enhance existing functionality and are fast to implement label Apr 9, 2024
@Miraeld Miraeld added this to the 3.16 milestone Apr 9, 2024
@Miraeld Miraeld requested a review from a team April 9, 2024 03:52
@Miraeld Miraeld self-assigned this Apr 9, 2024
@Miraeld Miraeld changed the base branch from develop to feature/lcp-above-the-fold-optimization April 9, 2024 03:52
Copy link
Contributor

@MathieuLamiot MathieuLamiot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@Miraeld Miraeld merged commit 7947b65 into feature/lcp-above-the-fold-optimization Apr 16, 2024
9 checks passed
@Miraeld Miraeld deleted the enhancement/6527-images-intersecting-viewport-considered-as-lcp-candidates branch April 16, 2024 12:57
@Mai-Saad
Copy link
Contributor

Mai-Saad commented Apr 23, 2024

Test Note:

@Miraeld
Copy link
Contributor Author

Miraeld commented Apr 24, 2024

@Mai-Saad Not sure why but I can't add any result on the test plan,
But here you go:

Step 1

Works perfectly.

Step 2

Works perfectly.

Step 3

Works perfectly.

@Mai-Saad
Copy link
Contributor

@Mai-Saad Not sure why but I can't add any result on the test plan,

As that was just test case not inside test plan (as it's single test). Thanks for sharing the results 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.16 enhancement - Images intersecting the viewport should be considered as LCP candidates
3 participants