Post Comments block: Whitespace on top of warning message

[ockham]

Description

Users that still have the now-deprecated Post Comments block in one of their templates (or elsewhere) will encounter a warning, suggesting them to use the Comments Query Loop block instead.

That warning has some ugly extra whitespace on top of it.

Step-by-step reproduction instructions

1. The Post Comments block is currently hidden from the inserter. You need to remove the following line (and rebuild Gutenberg) in order to show it:

   gutenberg/packages/block-library/src/post-comments/block.json

   Line 37 in 54fd3bf

   "inserter": false

2. Go to the Site Editor and edit the Single page template.
3. Insert the block named "Post Comments (deprecated)".
4. Note the warning that appears on top of the block.
5. Specifically, notice the ugly extra whitespace on top of that warning😬

Screenshots, screen recording, code snippet

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[amustaque97]

Hi, I would love to take this issue up. As per my investigation, this issue is theme specific. In twentytwnetytwo theme we have added a custom spacing. https://github.com/WordPress/twentytwentytwo/blob/trunk/theme.json#L141

I'm not sure how to fix this. Shall I directly add property padding-top: 0 !important in packages/block-library/src/post-comments/style.scss file? Or what would be the right approach to fix it!? I would appreciate your help.

I changed the theme to tt1 block. I don't any space there.

I'm sharing screenshots for reference.

TT1 Block

Twenty Twenty Two

[ockham]

Thanks @amustaque97, that's helpful information!

I'm not sure how to fix this. Shall I directly add property padding-top: 0 !important in packages/block-library/src/post-comments/style.scss file?

We shouldn't add ! important, these are always problematic (and typically also fragile) 😅

Or what would be the right approach to fix it!? I would appreciate your help.

It's a tricky question 😅 -- when I filed this issue, I wasn't really sure how we'd tackle it. I tried to think a bit more about it now and had an idea:

Both this issue and #40536 stem from the fact that the Warning is displayed "inside" of the block.

But there’s precedent for Warnings that are displayed outside (such as the "Invalid Block markup" warning), and they don’t suffer from those issues. So I'd like to try that; I think there's hook for that. I'll have to dig a bit though to find the right one.

[amustaque97]

Hi thank you for reply. I'm also going through all the related issues and PRs. I would like to add here is though we're trying to encapsulate warning block styling... In this issue, I thing we need some different fix. What I mean to say is I moved the Warning block outside of the div, just to try out. <Warning> block is at its correct place but the post-comments editor is still having padding-top which I believe shouldn't be there. I'm sharing a screenshot for reference.

Let me know what you guys think?
cc @michalczaplinski @DAreRodz @ockham

Screenshot

[ockham]

Yeah, that's a good point @amustaque97. In this case, I think we should file a bug against the Twenty Twenty-Two theme. Since TT2 is now part of Core, that probably means filing a Trac ticket.

I'm still not entirely sure what the correct fix (at theme level) will be. This is one of the first actual block themes, and I assume there's a reason for the top padding. In traditional themes, the layout is kinda fixed, so when applying some styling to change the space between layout elements, we always know which other elements are adjacent to any given element. This allows us to use CSS selectors to define spacing between elements in a pretty fine-grained way that will not leak to other occurrences of the same layout element.

This is different with block themes, were the user can re-arrange blocks, or insert the block in a totally different place, and the block should still look "correct". The challenge for theme authors is twofold: First of all, make sure that the theme looks good "out-of-the-box", i.e. without any user modifications. Secondly, the blocks should still look good when moved around or inserted elsewhere.

Thinking about this, I can't see too many possible solutions.

1. Analyze the theme to find out in which places the Post Comments block is used. If it turns out that it's always preceded by the same other block, we could consider adding a bottom margin to that block, and remove the top margin from Post Comments. (It's quite likely that that would have unwanted side effects for that other block, when used elsewhere.)
2. Use a Spacer block before the Post Comments block. This seems like a somewhat promising solution. The downside is that we can't set its height to a relative var like --wp--custom--spacing--small; we'd have to assign a fixed value to it instead.
3. Keep the default top padding, but add an Inspector Control to allow the user to change it. This would make some sense to me.

Curious to hear your thoughts 🙂

[amustaque97]

Analyze the theme to find out in which places the Post Comments block is used. If it turns out that it's always preceded by the same other block, we could consider adding a bottom margin to that block, and remove the top margin from Post Comments. (It's quite likely that that would have unwanted side effects for that other block, when used elsewhere.)

This is the particular commit when team added the padding-top property. I'm not well versed with the themes 😅 so no idea how we can fix it in the themes.

Use a Spacer block before the Post Comments block. This seems like a somewhat promising solution. The downside is that we can't set its height to a relative var like --wp--custom--spacing--small; we'd have to assign a fixed value to it instead.

I agree, as you said it has its own limitation.

Keep the default top padding, but add an Inspector Control to allow the user to change it. This would make some sense to me.

This solution makes the best sense to me since user is free to decide the padding. He can assign his own value without any hiccups. Furthermore, it would save them from going to theme.json and can making directly changes using Inspector Control. Point to note here is that while we implement this we might need to use !important. During my observation, if we add padding-top property on block level, it is not getting applied :(

[ockham]

This is the particular commit when team added the padding-top property. I'm not well versed with the themes 😅 so no idea how we can fix it in the themes.

Thanks, great find!

Use a Spacer block before the Post Comments block. This seems like a somewhat promising solution. The downside is that we can't set its height to a relative var like --wp--custom--spacing--small; we'd have to assign a fixed value to it instead.

I agree, as you said it has its own limitation.

Looks like the PR you found was basically doing the opposite of that -- removing Spacer blocks in favor of that hard-wired padding 😅

Keep the default top padding, but add an Inspector Control to allow the user to change it. This would make some sense to me.

This solution makes the best sense to me since user is free to decide the padding. He can assign his own value without any hiccups. Furthermore, it would save them from going to theme.json and can making directly changes using Inspector Control.

👍 I've now filed https://core.trac.wordpress.org/ticket/55704 to suggest this change 🙂

Point to note here is that while we implement this we might need to use !important. During my observation, if we add padding-top property on block level, it is not getting applied :(

Not sure what you mean by "add[ing] padding-top property on block level"? 🤔 I thought there's some existing block-supports mechanism for padding that we could use, is that what you tried?

[amustaque97]

Not sure what you mean by "add[ing] padding-top property on block level"? 🤔 I thought there's some existing block-supports mechanism for padding that we could use, is that what you tried?

NO I didn't get chance to try it out. I got confused here! I mixed my previous findings in the last comment 😅 . Please ignore.

[ockham]

@amustaque97 Would you like to give it a try? I think we basically need a PR filed against the wordpress-develop repo that enables UI controls for padding in TT2's theme.json (see docs) 🙂

[amustaque97]

Hey @ockham, can we close this issue? Since you already opened an issue on the core side.

[ockham]

Hey @ockham, can we close this issue? Since you already opened an issue on the core side.

Yeah, should be okay to close I guess 👍