Move PictureLayout to grid - #16251
Conversation
f33830a to
e33629d
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
e2f5162 to
d9fba56
Compare
6729bd2 to
53aad3f
Compare
JamieB-gu
left a comment
There was a problem hiding this comment.
Overall approach looks good. Comments mostly about application of CSS and the grid rules.
| headline: { | ||
| mobile: 'grid-row: 2;', | ||
| tablet: 'grid-row: 2;', | ||
| desktop: `grid-row: 2; ${grid.between('centre-column-start', 'right-column-end')};`, |
There was a problem hiding this comment.
I think you're asking this element to span both columns here, then on the element inside it you're setting a max-width to ensure it's essentially the same width as the centre column? Would it be easier to just set grid.column.centre here (which I think is the default) and remove that max-width, or am I missing something about the design?
There was a problem hiding this comment.
I've made some comments about how you probably don't need to repeat certain rules at later breakpoints because earlier ones still apply. But you're doing this a lot here so I'm wondering if there's a reason that I'm missing? If so, please ignore most of those comments!
9877fdf to
ebfdf4a
Compare
|
Please rebase this branch against
Please rebase this branch against |
5cab1a2 to
bbe6b1b
Compare
021969b to
367f7ed
Compare
367f7ed to
f4a4d49
Compare
f4a4d49 to
0494218
Compare
0494218 to
23869c4
Compare
23869c4 to
68f19e8
Compare
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
155c92d to
c75a9d3
Compare

Continuing on my grid rampage of #1542, #16119, and #16133 this absorbs PictureLayout into the new grid system for content pages. This one's a smidge more involved due to the avatar pics that sometimes render (though cracking that here paves the way for CommentLayout to move over too).
Screenshots