Add CLS to metric graphs

[patrickhulce]

Describe the solution you'd like CLS has a major weight in the score, is a Core Web Vital, and should be visually trackable in the dashboard. No design was ever created for it given that it was added to Lighthouse after LHCI was a thing and it's on another scale compared to all the other metrics.

Describe alternatives you've considered None.

[dkarzon]

I made a start on this but I wasn't able to get the project running locally yet 😞

https://github.com/GoogleChrome/lighthouse-ci/compare/main...dkarzon:cls-graph

This should be the API and UI components of it so new builds will have the statistics for it generated and the graphs are added to the dashboard but I wasn't sure how to do the database migration part to get the statistics for existing builds and I haven't been able to test it.

[patrickhulce]

Thanks very much @dkarzon! I think the complexity here though is in how to design the dashboard to avoid the odd numbered graph issues while still visualizing on smaller desktop viewport sizes.

[vikasrohit]

@patrickhulce what do you mean by odd numbered graph issues here?

[patrickhulce]

@patrickhulce what do you mean by odd numbered graph issues here?

Simply that we've got a nice two column layout going at the moment that fits nicely in a single viewport. If we just added a 3rd graph with CLS it would be thrown off a bit and require some scrolling. That's the simple solution here and given the lack of movement here maybe that's just the best course of action for now 🤷

[vikasrohit]

@patrickhulce I am able to get upto this point where I can render the graph for CLS and as first graph has multiple metrices rendered, I tried to move it down to occupy more space.

However, there seems to be an existing bug which prevents the tooltip (hover-card) from being rendered for the graph which is rendered last (in this screenshot graph with FCP/LCP/TTI/SI). If I move the CLS graph to the last, it stops rendering hover-card for it. I am still trying to debug it.

[vikasrohit]

@patrickhulce did you get chance to look at this layout?

[patrickhulce]

if we flip the FCP/LCP graph above the other two then LGTM :)

[vikasrohit]

if we flip the FCP/LCP graph above the other two then LGTM :)

Yes, that also looks good and infact I tried that first. This layout I tried to see if there is bug already existing or is it introduced because of my changes. The bug I am talking about is:

However, there seems to be an existing bug which prevents the tooltip (hover-card) from being rendered for the graph which is rendered last (in this screenshot graph with FCP/LCP/TTI/SI). If I move the CLS graph to the last, it stops rendering hover-card for it. I am still trying to debug it.

[vikasrohit]

@patrickhulce do we have any guide for how to fix unit tests for these changes? I am seeing some unit tests failing after introduction of new graph.

[patrickhulce]

Which unit tests? Link to log output would be great

[vikasrohit]

@patrickhulce I am just trying to run npm run test after my changes and it is breaking some of the existing tests.

[patrickhulce]

I understand that, but I can't help unless I know which tests and how they are being broken :)

[vikasrohit]

@patrickhulce here are few of the tests (snapshots mostly) which are failing:

[patrickhulce]

OK and have you opened the diff to see if what has changed is expected? If you're changing the UI, updating snapshots is normal :)

[vikasrohit]

I didn't try to open diff yet as I didn't realize that I can see the diff image as well. On the second point, is there any guide how to update snapshots?