report(metrics): use css grid so metrics are aligned #10695
Conversation
|
there needs to be a 😍 reaction on PRs :) |
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
| } | ||
| .lh-metrics-container { | ||
| display: grid; | ||
| grid-template-rows: 1fr 1fr 1fr; |
There was a problem hiding this comment.
dunno a lot about grid but does this need to be repeated 3 times? what if another metric is added via custom config or w/e. do we need changes here?
There was a problem hiding this comment.
right, a 7th metric would make a new column and be weird ...
if instead i used grid-template-columns: 1fr 1fr (and drop the grid-flow: columns so it defaults to rows) the order is no longer we want:
we forced this order before by using two divs, but that didn't allow for alignment.
There was a problem hiding this comment.
idk how to solve this other than keeping as-is and being ok with this in for some reason there is another metric in the LHR
I think we'd redesign this whole thing if we had >6 metrics here tho..
There was a problem hiding this comment.
another solution: interleave the creation of these lh-metric divs such that the rendering order is good. like 1 4 2 5 3 6 ...
There was a problem hiding this comment.
in other words, css grid doesnt allow for letting elements fill up a column 1 first, column 2 second, and then not create a 3rd column (what would that css even look like...)
There was a problem hiding this comment.
gotcha. IMO with 7 metrics it doesnt look that bad.
I'm fine with keeping the css as you have it.
| .lh-column:first-of-type { | ||
| margin-right: 8px; | ||
| @media screen and (min-width: 640px) { | ||
| .lh-metric:nth-last-child(-n+2) { |
This reverts commit d8d386d.
This reverts commit d8d386d.
before:
after: