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

feat: Attempt to display non-zero value on Web Vitals #29358

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

Conversation

rafaeelaudibert
Copy link
Member

@rafaeelaudibert rafaeelaudibert commented Feb 28, 2025

Rather than blindly displaying the last Web Vitals value, let's instead display the last non-zero value in an effort to avoid display zero just after midnight when the last period has no data

This isn't true for CLS because well-optimized sites will actually have a 0 value, so let's do 0 for them

Also, display a slightly-too-prominent-but-intended-to-reduce-confusion message under those cards
image

Rather than blindly displaying the last Web Vitals value, let's instead display the last non-zero value in an effort to avoid display zero just after midnight when the last period has no data

This isn't true for CLS because well-optimized sites will actually have a 0 value, so let's do 0 for them
@rafaeelaudibert rafaeelaudibert requested review from lricoy, robbie-c and a team February 28, 2025 13:43
@rafaeelaudibert rafaeelaudibert enabled auto-merge (squash) February 28, 2025 13:43
@rafaeelaudibert rafaeelaudibert changed the title Attempt to display non-zero value on Web Vitals feat: Attempt to display non-zero value on Web Vitals Feb 28, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR improves Web Vitals metrics display by showing the last non-zero value for most metrics (except CLS where zero is valid) to avoid misleading zeros when recent data is missing.

  • Modified getMetric in frontend/src/queries/nodes/WebVitals/definitions.ts to return the last non-zero value for INP, LCP, and FCP metrics while preserving zero values for CLS
  • Added explanatory text below the metrics grid stating "Metrics above are from the last day in the selected time range" to provide context
  • Wrapped the WebVitalsTab components in a flex container with proper styling for better layout organization
  • Implemented special handling for CLS metric to show zero values since they indicate good performance

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

github-actions bot commented Feb 28, 2025

Size Change: 0 B

Total Size: 9.73 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 9.73 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Note that the values are from the last day in the period selected, this wasn't clear enough before, some people thought it was the average - but that just doesn't make any sense

Update UI snapshots for `chromium` (3)

Update UI snapshots for `chromium` (1)
@rafaeelaudibert rafaeelaudibert force-pushed the display-slightly-more-useful-number-at-the-top-for-web-vitals-if-zero branch from ad7ddb8 to 6779b45 Compare February 28, 2025 21:35
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 3)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

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

Successfully merging this pull request may close these issues.

2 participants