report: full bleed image thumbnails #9238
Conversation
| const strValue = details; | ||
| element.src = strValue; | ||
| element.style.backgroundImage = `url("${strValue}")`; |
There was a problem hiding this comment.
any particular reason for background image instead of leaving as image with object-fit: cover;?
There was a problem hiding this comment.
@exterkamp points out a very good reason to keep them image elements :)
fixed. |
|
I think the tests might need some love after the move back to img :) |
| @@ -132,6 +132,7 @@ | |||
| --stackpack-padding-horizontal: 10px; | |||
| --sticky-header-background-color: var(--report-background-color); | |||
| --table-higlight-background-color: hsla(0, 0%, 75%, 0.1); | |||
| --image-preview-size: 48px; | |||
There was a problem hiding this comment.
LGTM all nits addressed!
Some other things I noticed that are out of scope of this PR, but def report things:
-
The word wrap messes with img spacing now more obviously in narrow tables (notice how the bottom images are full bleed but the others aren't)
-
Mobile doesn't scroll horizontally? It cuts off some table...
This was part of the design system that we didnt do yet. #8185
From the design:
And here's the before and after, with this PR:
The table actually gets shorter due to how the padding on these thumbnails was working before. A win for density and a nice visual improvement, too!