Separator: Remove border-bottom property

[tjcafferkey]

What?

The separator block was using a top and bottom border of 1px to create the horizontal line. This change removes the bottom border and increases the border-top to 2px.

Fixes #31424

Why?

There was some occasional visual gap happening between the top and bottom border within the editor (was fine on the frontend). It came up in an issue regarding patterns on WooCommerce found here woocommerce/woocommerce-blocks#11304 (comment) but can be reproduced on any site.

How?

Removing the bottom border altogether means this gap cannot appear.

Testing Instructions

There is no real rhyme or reason that I have found to reproducing this bug, it appears random but happens often enough especially when working with patterns I've found.

1. Open a post or page.
2. Insert a separator block.
3. Insert a pattern of choice.
4. Insert another separator block.
5. Keep repeating steps 2-4 and try using different combinations (e.g. different blocks, multiple separators one after another etc).
6. No double line should appear when using the separator. Check these changes on the frontend also to ensure no regressions have occured.

Testing Instructions for Keyboard

Screenshots or screencast

Here is a screenshot of it happening within the editor. Another example can be found here woocommerce/woocommerce-blocks#11304 (comment)

[github-actions]

Warning: Type of PR label error

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
• Labels found: [Block] Separator.

Read more about Type labels in Gutenberg.

[tjcafferkey]

@ajitbohra is there anything further needed from me, or anything I can do to help progress this PR? Thanks.

[tjcafferkey]

@skorasaurus sorry for the ping, but is there anything I can do to help progress this PR?

[opr]

At a glance this looks like it should be fine. I'm trying to think if there's any way removing the bottom border could negatively affect the display of someone's site, e.g. if they actually desire the double line separator and have added some padding or a height to .wp-block-separator now there'd just be a single line with a gap under it.

Probably a very far outlying edge case but that's the only issue I can think of with this change.

[skorasaurus]

I haven't tested the PR yet but this issue probably fixes #31424

[tjcafferkey]

@skorasaurus is there anything I can do to progress this PR?

[skorasaurus]

Hi @tjcafferkey

Thanks for reaching out. You're not doing anything wrong (perhaps if you could find a way to consistently reproduce the bug and its cause might inspire some more confidence that your proposed fix would really fix the issue in all cases and not unintentionally break something else; but that may be a bit nitpicking).

I really understand: there are hundreds of pull requests that need to be reviewed, and (in my opinion) paid core contributors (specifically ones paid by their employers to work on Gutenberg development; I am not) could spend a bit more (not much, just ) to review existing PRs (possibly incentivize more to potential contributors that their work will be included), reduce new issues that would be fixed if there's already an outstanding PR for it)

cc @annezazu who can maybe help facilitate this.

(I realize the irony in this in that the time that in writing this, I could have spent some of this time testing the PR and reviewing the code but it (the amount of outstanding PRs) is something I've had in my mind for a long time.

[tjcafferkey]

Thanks for your honest reply @skorasaurus. I understand the problem you're facing and appreciate you taking the time to write such a considered response explaining the situation. Hopefully things will start improving soon.

[annezazu]

👋🏼 Been heads down on 6.5 efforts which is likely also what's going on here. We're in a key window (right now, not earlier) with a month out from beta 1 for the next release so reviews are harder to come by. With that said, I'd recommend reviewing what's shared here. In the meantime, I looked through PRs done by folks for the separator block. @carolinan might you be able to help move this forward? I saw you had some PRs in the last year related to the block itself.

[carolinan]

As discussed in the linked issues, these CSS problems are never as easy as they seem.
Updating styles affects all blocks that have already been placed, in classic themes and block themes. So it has to be carefully considered if it is worth updating when it creates visible layout changes on these live websites.
What is a bug for someone is a feature for someone else, and there will always be extenders that will need to update their CSS to adapt to the change.

[carolinan]

We have to be careful which themes we are testing the PR with, as Twenty Twenty-Four for example adds this styling that will override the defaults:

			"core/separator": {
				"border": {
					"color": "currentColor",
					"style": "solid",
					"width": "0 0 1px 0"
				},

• For any theme that adds no extra styling and opts-in to "wp-block-styles" which includes the theme.scss, the theme.csss file style overrides the style from style.scss and there will be no visible difference.
• For any theme that adds no extra styling and does not opt-in to wp-block-styles, there is no visible difference.
• Twenty Twenty which has a customized separator with a set height, also overrride the border style so there is no visible difference.
• Twenty Twenty-One has a custom block style called "thick" which overrides the bottom border, and because the theme also opts-in to wp-block-styles, there is no visible difference.

[carolinan]

For any theme (or plugin) that sets a height on the hr or separator block the PR is of course a visible difference but it is not possible to determine how many sites that would be and I think the PR should be merged.

For example the theme Astra does have an HR with a height, but already removes the height for the separator block and there is no visible difference with the PR.
Ocean WP opts-in to wp-block-styles.
In Kadence there is no visible difference, the height is set to 0 and their identical 2px border is just moved from the bottom to the top.

[tjcafferkey]

@carolinan thanks for this approval. Unfortunately I'm not authorized to merge the PR. Anything further required from me?