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

Add a ‘needs improvement’ threshold for core web vitals; improve icons. #2

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

adamsilverstein
Copy link
Contributor

Core web vitals CLS, FID and LCP define both a "good" threshold and a "needs improvement" threshold (see web.dev). This PR seeks to recognize those thresholds and display a 'warning' icon when a metric needs improvement.

I left the green checkmark for good, added a yellow warning for needs improvement and changed the is-poor icon to a red X.

Looks something like this:

image

Copy link
Owner

@stefanjudis stefanjudis left a comment

Choose a reason for hiding this comment

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

One tiny nit. Thanks so much @adamsilverstein, I'm happy to 🚢 after a tiny adjustment. :)

@@ -8,15 +8,16 @@ const METRIC_CONFIG = new Map([
[
'CLS',
{
threshold: 0.1,
goodThreshold: 0.1,
needsImprovementThreshold: 0.25,
Copy link
Owner

Choose a reason for hiding this comment

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

Tiny nit here: what do you think of having a threshold property instead of these two.

{
  thresholds: {
    good: '',
    needsImprovement: '',
    poor: ''
  }
}

I think I'd like to avoid of having them all in the root object level. :)

@adamsilverstein
Copy link
Contributor Author

Tiny nit here: what do you think of having a threshold property instead of these two.

Great suggestion, I will adjust the PR to make it so.

@adamsilverstein
Copy link
Contributor Author

@stefanjudis in 5d0c4d1 (#2) I moved the thresholds to a root thresholds object as you suggested.

Please note there is no poor value because the way the logic works is: default score is poor; if metric has a needsImprovement setting and the value meets needsImprovement threshold, score is needsImprovement. if value meets the good threshold, score is good.

Copy link
Owner

@stefanjudis stefanjudis left a comment

Choose a reason for hiding this comment

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

That looks good to me. Thanks a bunch @adamsilverstein.

I'm gonna merge have a final look and will release a minor version. :)

@stefanjudis stefanjudis merged commit 8e7f73c into stefanjudis:main Sep 2, 2020
@stefanjudis
Copy link
Owner

And v1.1.0 released. :)

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.

None yet

2 participants