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

Logging lcp target #607

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AbdelrhmanMagdy
Copy link
Collaborator

No description provided.

@AbdelrhmanMagdy AbdelrhmanMagdy marked this pull request as ready for review September 24, 2024 13:06
@@ -42,6 +42,12 @@ export function max(metric: Metric, value: number): void {
}
}

export function logTarget(event: Event, target: number): void {
if(event == Event.LargestPaintTarget){
Copy link
Contributor

Choose a reason for hiding this comment

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

triple ===

@@ -81,6 +83,10 @@ function process(entries: PerformanceEntryList): void {
break;
case Constant.LCP:
if (visible) { metric.max(Metric.LargestPaint, entry.startTime); }
const targetNode = dom.get((entry as any).element)?.id
if(targetNode){
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, space between if and (

@@ -81,6 +83,10 @@ function process(entries: PerformanceEntryList): void {
break;
case Constant.LCP:
if (visible) { metric.max(Metric.LargestPaint, entry.startTime); }
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to only log targetNode if it is visible? not sure why we log the metric when visible, but the rest of it regardless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants