core: add new CLS (all frames) to hidden metrics audit #12476
Conversation
There was a problem hiding this comment.
impl LGTM
I have the open naming question mostly because I'm not sure where this PR fits in with the overall plan for new CLS. If it's a very temporary thing that will disappear by 8.0, then as-is WFM. If it's being engraved permanently in metrics.js today, then let's chat :)
| * @return {number} | ||
| */ | ||
| static newCumulativeLayoutShift(layoutShiftEvents) { | ||
| const gapµs = 1_000_000; |
There was a problem hiding this comment.
While this was super neat to see, I've seen enough people mistakenly use µs thinking it's "fancy" millisecond or nanosecond to be minorly concerned about this (not to mention problematic with the fact that all of our camelcase millisecond Ms are actually indistinguishible from capital Mu Ms) 😆
how sad would it be to use?
| const gapµs = 1_000_000; | |
| const gapMicroseconds = 1_000_000; |
There was a problem hiding this comment.
how sad would it be to use?
not sad at all :)
(not to mention problematic with the fact that all of our camelcase millisecond
Msare actually indistinguishible from capital MuMs)
ah, see, there's the problem. You're still writing M when clearly you mean Μ :D
types/artifacts.d.ts
Outdated
| @@ -789,6 +789,7 @@ declare global { | |||
| layoutShiftMaxSessionGap1sLimit5s: number, | |||
| layoutShiftMaxSliding1s: number, | |||
| layoutShiftMaxSliding300ms: number, | |||
| newCumulativeLayoutShiftAllFrames: number, | |||
There was a problem hiding this comment.
I didn't expect new to be part of the internal name :)
layoutShiftMaxSessionGap1sLimit5sAllFrames? :D
There was a problem hiding this comment.
layoutShiftMaxSessionGap1sLimit5sAllFrames is good
ha, no, that's very much what I was planning, but I guess I didn't discuss with others more than here and in #12046. Basically live in |
Cool beans, that's what I figured, but wanted to double check just in case :) |
first checkbox of #12350
It should still be easy to copy and paste out of the variants file into the real metric file when the time comes. Doing it now in the
metricsaudit so the tail end of the May HTTP Archive run can pick up some CLS AF data with the new CLS definition.