Layout Grid: Fix vertical alignment, round 2 #254
Conversation
There was a problem hiding this comment.
Nice work @jasmussen, that's done the trick!
This is working nicely for me in the post and site editors, and on the front end. Top vertical alignment stretches the full height of the container, and middle and bottom alignment just to the height of the content:
I haven't been able to fault it on my test site, so LGTM! Just left a couple of tiny notes about whitespace in the style.scss diff 🙂
blocks/layout-grid/style.scss
Outdated
| @@ -168,7 +168,8 @@ | |||
| /** | |||
| * Parent column alignment | |||
| */ | |||
| .wp-block-jetpack-layout-grid { | |||
|
|
|||
| .wp-block-jetpack-layout-grid { | |||
There was a problem hiding this comment.
Tiniest of tiny nits: there's added whitespace at the beginning of this line
blocks/layout-grid/style.scss
Outdated
| @@ -185,9 +186,12 @@ | |||
| /** | |||
| * Individual column alignment | |||
| */ | |||
| .wp-block-jetpack-layout-grid-column { | |||
|
|
|||
| .wp-block-jetpack-layout-grid-column { | |||
There was a problem hiding this comment.
Tiniest of tiny nits: there's added whitespace at the beginning of this line
|
Your review was excellent, thanks so much. I'll try and keep an eye on any feedback around the columns not being full-height when vertically aligned; but since that's arguably been entirely broken until now, I don't expect a ton. I'll land this one, look at another quick fix, and then hopefully we can make a release. Thanks again. |
Alternative to #252. Fixes #250. A few things going on:
Together these present a bit of a challenge; if we wanted a full height colored background for a column with text bottom aligned, then it would have to be a flex container, as #252 does. But that breaks margin collapsing.
It seems we've supported full height columns, but never for an alignment other than top (please correct me if I'm wrong). And so the smaller fix in this PR adjusts the rules for that to match, and cleans things up a bit.
Site editor before (vertical alignments didn't work):
Site editor after (vertical alignments work, and columns are full height when top aligned):
Post editor and frontend also work the same:
In fixing this, I've noticed a few regressions with the appender and the inspector which I'll follow up on separately.