Bundled Themes: Add styling for blocks added in 5.8

[desrosj]

In testing the new blocks added in 5.8, I've found that a lot of them don't show correctly.

Uploading some screenshots of a few using Twenty Twenty-One, but all of the new blocks should be tested thoroughly.

[Joen]

Hey @desrosj, I'd like to help out with this if I can, to debug and see what the issue is, and push any block fixes if there are any, or help triage TT1 or other theme issues there alternatively.

I'm having some trouble entirely reproducing your screenshot. Best I can tell it includes a query loop in the editor, but I haven't been able to reproduce the full-wide thing. Can you list which blocks are problematic, and how? Perhaps share some demo content?

From a quick glance, the biggest issue that stood out to me was the separator block seemingly collapsing on the frontend. That appears to be a TT1 issue as can be seen here: ​https://cloudup.com/cB7CbBnAONt — specifically that the following block spacing rule is output in the editor, with no equivalent on the frontend:

.editor-styles-wrapper [data-block] {
    margin-top: var(--global--spacing-vertical);
    margin-bottom: var(--global--spacing-vertical);
}

I'll look for existing TT1 tickets to see if one already exists, and will create one otherwise.

In any case, let me know if I can help in any way, would be happy to.

[Joen]

I went looking for TT1 issues and pondered whether to open a new ticket. The primary issue is that the blanket [data-block] rule above targets every single block ever, inside the editor, regardless of whether it's horizontal or vertical or a child block or not.

The reason that rule exists is because the block editor itself, for classic themes, outputs such a margin. So while it's unfortunate that the theme's editor style adds specificity, it is nevertheless not unreasonable for the rule to exist.

It also seems like there actually _is_ a frontend equivalent to the rule, this one:

@media only screen and (min-width: 482px)
.site-main > article > *, .site-main > .not-found > *, .entry-content > *, [class*=inner-container] > *, .wp-block-template-part > * {
    margin-top: var(--global--spacing-vertical);
    margin-bottom: var(--global--spacing-vertical);
}

So the problem here is that that rule does not successfully target the separator block when it's used inside the Query Loop block. That seems like a small thing to fix. Let me know if you'd like to open a separate ticket for this, otherwise I'll keep it here.

[francina]

Hi @desrosj, I could do some testing, but I feel it would be better to have one ticket per theme + a list of blocks that need to be tested and potentially dummy content, so testers don't have to create the blocks manually.
Thanks!

[desrosj]

I'll open a new ticket for each theme with test scenarios. I think polishing the new blocks in Twenty Twenty-One and Twenty Twenty should be our goal for 5.8.

This PR aims to fix https://core.trac.wordpress.org/ticket/53389 and https://core.trac.wordpress.org/ticket/53398

[Joen]

I created a PR in ​https://github.com/WordPress/wordpress-develop/pull/1372 which I believe should fix both this ticket, and https://core.trac.wordpress.org/ticket/53398 as well, since both are related to margins not being output for blocks inside the query loop.

I don't love how TT1 in general handles block margins. Because it targets only blocks that are descendants of a particular parent, it adds extra specificity which is likely going to blocks that shouldn't necessarily have top/bottom margins inside. But changing this would require a bit of a refactor at this point. Instead I've explored using the :where selector to keep specificity low.

Here's before:

​https://cloudup.com/cwrO4SwqTG5

Here's after:

​https://cloudup.com/cnQp2UzKRCB

This is my first patch to TT1, so I'd appreciate a sanity check that I did it right. Running the build script for the SASS files generated a bunch of unrelated changes to whitespace in the CSS files — I included them for transparency, but I'm not entirely sure why they happened.

Here's the core of the code change, made in assets/sass/03-generic/vertical-margins.scss:

.site-main > article > *, // apply vertical margins to article level
.site-main > .not-found > *, // apply vertical margins to article level
.entry-content > *,
[class*="inner-container"] > *,
.wp-block-template-part > *,
.wp-block-post-template :where(li > *) { // using :where keeps specificity low.

	margin-top: calc(0.666 * var(--global--spacing-vertical));
	margin-bottom: calc(0.666 * var(--global--spacing-vertical));

	@include media(mobile) {
		margin-top: var(--global--spacing-vertical);
		margin-bottom: var(--global--spacing-vertical);
	}

	&:first-child {
		margin-top: 0;
	}

	&:last-child {
		margin-bottom: 0;
	}
}

[JeffPaul]

See related tickets...
Twenty Twenty One: #53398
Twenty Nineteen: #53428

[poena]

People have expressed that this is difficult to test because there is no list of which blocks to test.

This is not an official list of blocks that will be in 5.8. These are the blocks I found.

@youknowriad When will the list be posted publicly?

Site blocks:
Site title
Site tagline
Site logo

Post blocks:
Post title
Post date
Post featured image
Post content
Post excerpt
Post tags -variations
Post categories -variations

Archive blocks:
Archive title

Structural blocks:
Login / out
Page list

Post listing blocks:
Query Loop, alias Post List
Post Template (inner block)
Query pagination (Previous page, next page, inner blocks)

Here is a Gist with the list and a copy of each block and some basic testing instructions.
Copy the content to a page or post. ​https://gist.github.com/carolinan/356d263455513500c2fc1af825df615e

[youknowriad]

Hey @poena The list sounds correct and the gist there is very helpful, thanks for sharing.

I did publish the list of blocks here ​https://make.wordpress.org/core/2021/06/16/introducing-the-template-editor-in-wordpress-5-8/ (I didn't detail the Query related blocks though)

[poena]

Replying to youknowriad:

I did publish the list of blocks here ​https://make.wordpress.org/core/2021/06/16/introducing-the-template-editor-in-wordpress-5-8/ (I didn't detail the Query related blocks though)

Thank you!

[ryelle]

@Joen's PR fixes the vertical margin issue in Twenty Twenty-One (scenario-1-front.png​). I can't replicate the horizontal alignment in scenario-2-front.png​. I'm going to move some other Twenty Twenty-One feedback to #53398.

As for the blocks more generally…

• I noticed a lot of extra spacing in the grid pattern of Query Loop. It looks like I can control that padding in Twenty Twenty-One, but not Twenty Twenty.
• Grid & Offset patterns: It looks like each of the grid items are using main elements, but there should only be one main on a page. ​gutenberg/issues#31502 would help, but these patterns should probably be updated to div regardless.
• In the editor, the Post Template block defaults to regular width, even if the Query Loop block is full/wide, so it looks misaligned. On the frontend it works as expected, the content is shown full/wide width.

[ryelle]

Full width Query Loop, but Post Template still appears narrow

[ryelle]

I created a PR into gutenberg to swap out the tagName: main on those patterns: ​https://github.com/WordPress/gutenberg/pull/32867 - I can also upload a patch here for the same in core, if there won't be another GB sync.

[ryelle]

In 51201:

Twenty Twenty-One: Add margins around content in Post Template block.

Props desrosj, joen.
See #53389, #53398.

[onemaggie]

These changes look ok to me, the issue with the Post template being aligned left seems to come from ​https://github.com/WordPress/gutenberg/blob/c0ae9b28edc20b4262547297927abe007e255e24/packages/block-library/src/post-template/editor.scss#L4

[ryelle]

I think it's fine that it's aligned left, but it should be full width to fill out the Query Loop container. That's how it appears on the frontend in the default themes, and since I haven't told it to have a width, I'd expect it to expand to fill the space.

[desrosj]

Are there any more issues that need to be addressed here? Or can we close this one out in favor of separate tickets for each individual bug going forward?

[JeffPaul]

I concur on closing this and handling further iterations / additional bugs in subsequent tickets.