Blockbase: Add post-meta template part #4565

mikachan · 2021-09-09T11:38:26Z

Changes proposed in this Pull Request:

This PR adds a post-meta template part to Blockbase.

I've mostly copied the Seedlet Blocks, Mayland Blocks, and Skatepark post-meta. I've used the 'tiny' font size; I did try 'small' as well but I thought 'tiny' probably looked better overall.

@kjellr, I know you mentioned in the original issue that we could maybe just include the post date to start with. Let me know if you think we should use different content - for now, I've gone with: author, date, categories, tags.

We're missing the correct 'post-tag' SVG icon (I think) - I've used an outline version of the 'offer' Material icon, but I think the line thickness is slightly different from the other icons. I'm not sure how we usually create the SVGs, so just pointing out that this one probably needs creating/replacing!

Related issue(s):

Closes #4522.

MaggieCabrera · 2021-09-09T12:00:32Z

Blockbase was initially a clone of Blank Canvas, which is a child of Seedlet with a few changes, so I think copying what's on Seedlet blocks should be ok, unless Kjell thinks otherwise

mikachan · 2021-09-09T13:18:02Z

Blockbase was initially a clone of Blank Canvas, which is a child of Seedlet with a few changes, so I think copying what's on Seedlet blocks should be ok, unless Kjell thinks otherwise

Ok! I've removed the tags as this then matches Seedlet Blocks exactly, but happy to adjust either way.

kjellr · 2021-09-13T12:02:48Z

I think including tags is fine (I'm not sure why Seedlet Blocks doesn't display them? The original Seedlet does, we just don't use tags on our demo site). You can source the icon straight from Seedlet.

I'd also remove the separator:

Before	After

Also, in my testing, the comments are showing up really close to this new meta area. Not sure if that's just a bug with my test site?

mikachan · 2021-09-13T13:35:27Z

Thanks @kjellr! I've added the post tags back in, copied the post tag SVG over from Seedlet (this looks great, thanks for pointing me in the right direction), and removed the separator.

Seedlet Blocks doesn't include the tags in the post meta template. I could add them here too while I'm working in this area, if you think they should be there?

The comments aren't really close on my local dev, looks like they're inheriting the top margin from this selector: .wp-site-blocks > * + * -

They're using the --wp--style--block-gap variable, so I don't know if this is related to a Gutenberg update, or maybe my local version is out of date.. Will have a play with this now.

kjellr · 2021-09-13T14:39:51Z

Seedlet Blocks doesn't include the tags in the post meta template. I could add them here too while I'm working in this area, if you think they should be there?

Yeah, I think they should be there. Feel free to do it here if you'd like, but it could totally be a separate PR if that's easier.

kjellr · 2021-09-13T14:41:22Z

The comments aren't really close on my local dev, looks like they're inheriting the top margin from this selector: .wp-site-blocks > * + * -
They're using the --wp--style--block-gap variable, so I don't know if this is related to a Gutenberg update, or maybe my local version is out of date.. Will have a play with this now.

If we could make this at least calc(2 * var(--wp--style--block-gap)), that would be great.

mikachan · 2021-09-13T16:01:44Z

I've updated Seedlet Blocks so the post meta now includes the tags:

And I managed to replicate the lack of spacing above the post comments block (my Gutenberg was out of date). I've added a spacer block above the post comments on Blockbase that's the same height as calc(2 * var(--wp--style--block-gap)). I added it as a spacer as all the child themes use a spacer block above the comments, and it also means we don't need to change the CSS for Blockbase and all the children.

This is ready for another review, thanks!

kjellr · 2021-09-14T12:17:55Z

Thanks, @mikachan! Just one minor comment: Can we increase the gap between items to calc(2 * var(--wp--custom--margin--baseline)) in Blockbase? This helps it match the gap size that Seedlet & Blank Canvas use normally.

Before	After

After that, I think this is all set for now!

kjellr · 2021-09-14T13:29:53Z

Can we increase the gap between items...

For what it's worth, I think the theme needs to opt into the new gap feature for that to work? I'm not sure how to actually set the gap for the block though. 🤔

danieldudzic · 2021-09-14T13:51:52Z

Can we increase the gap between items...

For what it's worth, I think the theme needs to opt into the new gap feature for that to work? I'm not sure how to actually set the gap for the block though. 🤔

So, I have checked the following blocks: post-author, post-date and post-terms - and they seem to be missing support for gap in the core.

If my understanding is correct, gap makes an obvious sense with blocks that will be used in multiple instances together, like buttons or columns.

With post-author, post-date and post-terms being individual blocks I can see how gap might be tricky.

It would almost make sense to have just one block called post meta, and then have different parts inside of the block being post-author, post-date and post-terms. That would make gap super useful.

kjellr · 2021-09-14T14:07:55Z

I think you only need gap support on the Group block, right? Since that's the one that has the flex style applied to it?

mikachan · 2021-09-14T16:10:27Z

For what it's worth, I think the theme needs to opt into the new gap feature for that to work?

Thank you! I tried with spacers and then resorted to CSS... but setting blockGap to true in theme.json seems to have done the trick.

I've set the Blockbase gap size to calc(2 * var(--wp--custom--margin--baseline)), and then set all the children gap sizes to var(--wp--custom--gap--vertical), as this was the previous value in theme.json. However, blockGap wasn't previously enabled, so all the themes were using the default gap size of 0.5em (which comes from Gutenberg I guess). This means all gap sizes are bigger now - would it be better to set them all to the old default value so there's no change?

Also, I could only seem to get blockGap working if it was applied globally (styles.spacing.blockGap), and I couldn't get it to work if I tried to set it directly on a block (e.g. styles.blocks.core/post-meta.spacing.blockGap). Or I was doing it wrong!

They were all using --wp--custom--margin--vertical instead of --wp--custom--gap--vertical

kjellr · 2021-09-15T13:30:13Z

However, blockGap wasn't previously enabled, so all the themes were using the default gap size of 0.5em (which comes from Gutenberg I guess). This means all gap sizes are bigger now - would it be better to set them all to the old default value so there's no change?

I think it probably makes sense to keep the default size the same for now. I'm not sure how many places that gap property is used, so I think a more conservative approach is warranted here.

mikachan · 2021-09-15T17:09:31Z

Thanks! I've set all of the Blockbase children gap sizes to 0.5em. Ready for another review.

scruffian · 2021-09-15T17:51:50Z

This now looking a bit odd on Seedlet:

Could all the children just inherit from Blockbase?

mikachan · 2021-09-16T10:50:29Z

Could all the children just inherit from Blockbase?

Yeah I think they can. I didn't do this originally as I only tried var(--wp--custom--gap--vertical) globally, which looked too big on most themes. I've just changed all themes to use calc(2 * var(--wp--custom--gap--baseline)), the same as Blockbase, and this seems to work well across them all. I'm not sure why I didn't try this before, seems like the obvious choice out of the gap sizes! Here's what this is looking like after this change:

Blockbase:

Seedlet Blocks:

Mayland Blocks:

Quadrat:

(This doesn't affect Skatepark and Geologist.)

kjellr · 2021-09-16T11:47:11Z

This looks great. One last question: Is it possible to specify a different vertical/horizontal gaps?

If these meta items wrap, it uses the same horizontal gap for the vertical spacing:

I'd prefer if the vertical spacing was half of that:

If that's not possible yet, it's ok, and we can merge this as-is for now. Thank you!

mikachan · 2021-09-16T14:12:25Z

Is it possible to specify a different vertical/horizontal gaps?

From what I can tell, the new blockGap feature only supports one value, and can't yet split out different horizontal and vertical gaps. It looks like there's an ongoing issue for it though: WordPress/gutenberg#34529 (and there's an issue from @scruffian here too: WordPress/gutenberg#34347)

We could set the vertical gap for the post-meta with CSS until this feature is available? Although it looks like it's tricky to overwrite the specific .wp-container-ID styles, so it might have to be something like:

.post-meta {
	row-gap: var(--wp--custom--gap--baseline) !important;
}

kjellr · 2021-09-16T14:54:04Z

Maybe we could set it via CSS just for the post meta?

This is now applied via theme.json (as the same value)

mikachan · 2021-09-16T15:48:07Z

Yeah I think this works, this is how it looks:

Blockbase:

Seedlet Blocks:

Mayland Blocks:

kjellr

Left one minor comment, but otherwise I think this is good to go. Thank you! 🙌

kjellr · 2021-09-16T21:37:38Z

blockbase/sass/post/_meta.scss

@@ -1,4 +1,6 @@
 .post-meta {
+	row-gap: var(--wp--custom--gap--baseline) !important;
+


Might make sense to add a code comment here to explain why we're using !important. And we can also probably remove that empty line below.

scruffian · 2021-09-17T11:00:12Z

blockbase/sass/post/_meta.scss

@@ -1,4 +1,5 @@
 .post-meta {
+	row-gap: var(--wp--custom--gap--baseline) !important; // !important is needed to override the unique .wp-container-ID classes


It would be good to link the GB issue that suggests different vertical and horizontal block gaps, so we can remove this when that gets fixed.

Ah yeah good idea, will sort this now

scruffian

LGTM

mikachan added 2 commits September 9, 2021 10:57

Blockbase: add post-meta template part

dd9509e

Blockbase: add post-tag icon

f85b21c

mikachan added the [Theme] Blockbase label Sep 9, 2021

mikachan self-assigned this Sep 9, 2021

mikachan requested review from kjellr and a team September 9, 2021 11:39

Blockbase: remove tags from post meta

0f0ee72

mikachan added 3 commits September 13, 2021 14:17

Blockbase: add post tags to post meta

d2633c4

Blockbase: add post tag SVG from Seedlet

604c4c9

Blockbase: remove separator from post meta

ec17fde

mikachan added 2 commits September 13, 2021 16:52

Blockbase: add spacer above post comments block

f843200

Seedlet Blocks: add post tags to post meta

babed04

Blockbase: enable blockGap & set gap size for all children

9178038

mikachan added 2 commits September 14, 2021 17:11

Merge branch 'trunk' into add/blockbase-post-meta

7341c6f

Blockbase children: sync up child-theme.json files

01acd6c

They were all using --wp--custom--margin--vertical instead of --wp--custom--gap--vertical

Blockbase children: change gap size to GB default of 0.5em

2e2a1a2

Blockbase & co: use gap baseline across BB & all children

88fe587

mikachan added 2 commits September 16, 2021 16:42

Blockbase: add row-gap value to post-meta

cda1fc1

Skatepark: remove gap value

b3b5683

This is now applied via theme.json (as the same value)

kjellr approved these changes Sep 16, 2021

View reviewed changes

Blockbase: add comment next to use of !important

3f6bbe7

scruffian reviewed Sep 17, 2021

View reviewed changes

Blockbase: add link to GB issue around block gaps

1d45572

scruffian approved these changes Sep 17, 2021

View reviewed changes

Merge branch 'trunk' into add/blockbase-post-meta

14339d4

scruffian merged commit d8926ce into trunk Sep 20, 2021

scruffian deleted the add/blockbase-post-meta branch September 20, 2021 08:51

kjellr mentioned this pull request Sep 20, 2021

Add post meta to Geologist Theme #4663

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blockbase: Add post-meta template part #4565

Blockbase: Add post-meta template part #4565

mikachan commented Sep 9, 2021

MaggieCabrera commented Sep 9, 2021 •

edited

Loading

mikachan commented Sep 9, 2021

kjellr commented Sep 13, 2021

mikachan commented Sep 13, 2021

kjellr commented Sep 13, 2021

kjellr commented Sep 13, 2021

mikachan commented Sep 13, 2021

kjellr commented Sep 14, 2021

kjellr commented Sep 14, 2021

danieldudzic commented Sep 14, 2021

kjellr commented Sep 14, 2021

mikachan commented Sep 14, 2021 •

edited

Loading

kjellr commented Sep 15, 2021

mikachan commented Sep 15, 2021

scruffian commented Sep 15, 2021

mikachan commented Sep 16, 2021

kjellr commented Sep 16, 2021 •

edited

Loading

mikachan commented Sep 16, 2021

kjellr commented Sep 16, 2021

mikachan commented Sep 16, 2021

kjellr left a comment

kjellr Sep 16, 2021

scruffian Sep 17, 2021

mikachan Sep 17, 2021

scruffian left a comment

		@@ -1,4 +1,6 @@
		.post-meta {
		row-gap: var(--wp--custom--gap--baseline) !important;

		@@ -1,4 +1,5 @@
		.post-meta {
		row-gap: var(--wp--custom--gap--baseline) !important; // !important is needed to override the unique .wp-container-ID classes

Blockbase: Add post-meta template part #4565

Blockbase: Add post-meta template part #4565

Conversation

mikachan commented Sep 9, 2021

Changes proposed in this Pull Request:

Related issue(s):

MaggieCabrera commented Sep 9, 2021 • edited Loading

mikachan commented Sep 9, 2021

kjellr commented Sep 13, 2021

mikachan commented Sep 13, 2021

kjellr commented Sep 13, 2021

kjellr commented Sep 13, 2021

mikachan commented Sep 13, 2021

kjellr commented Sep 14, 2021

kjellr commented Sep 14, 2021

danieldudzic commented Sep 14, 2021

kjellr commented Sep 14, 2021

mikachan commented Sep 14, 2021 • edited Loading

kjellr commented Sep 15, 2021

mikachan commented Sep 15, 2021

scruffian commented Sep 15, 2021

mikachan commented Sep 16, 2021

kjellr commented Sep 16, 2021 • edited Loading

mikachan commented Sep 16, 2021

kjellr commented Sep 16, 2021

mikachan commented Sep 16, 2021

kjellr left a comment

Choose a reason for hiding this comment

kjellr Sep 16, 2021

Choose a reason for hiding this comment

scruffian Sep 17, 2021

Choose a reason for hiding this comment

mikachan Sep 17, 2021

Choose a reason for hiding this comment

scruffian left a comment

Choose a reason for hiding this comment

MaggieCabrera commented Sep 9, 2021 •

edited

Loading

mikachan commented Sep 14, 2021 •

edited

Loading

kjellr commented Sep 16, 2021 •

edited

Loading