Skip to content
This repository has been archived by the owner on Dec 2, 2020. It is now read-only.

Editor style for spacing overrides has unintended consequences #857

Closed
jasmussen opened this issue Nov 19, 2020 · 2 comments
Closed

Editor style for spacing overrides has unintended consequences #857

jasmussen opened this issue Nov 19, 2020 · 2 comments

Comments

@jasmussen
Copy link

Describe the bug
There's a section of code in the TwentyTwentyOne editor style, it appears to both exist in style-editor.css (lines 1883-1897) and ie-editor.css (lines 2547-2561) which appear inteded to override the default spacing of blocks. This:

/**
 * Spacing Overrides
 */
[data-block] {
	margin-top: 30px;
	margin-bottom: 30px;
}
[data-block] [data-block]:first-child {
	margin-top: 0;
}
[data-block] [data-block]:nth-last-child(2) {
	margin-bottom: 0;
}

Specifically the rules that override margin-top and margin-bottom have unintended consequences. Here's the social links block:

issue

Notice how the first link button bumps upwards, the 2nd bumps downwards.

To Reproduce
Steps to reproduce the behavior:

  • Run the theme with the latest Gutenberg plugin installed
  • Insert a social links block
  • Observe how the first and 2nd to last elements are incorrectly sized.

Expected behavior
The theme should not affect margins in this way.

Screenshots

Here's a little inspecting showing how the style affects the block:

inspecting

Editor version (please complete the following information):

  • WordPress version: 5.6-beta2-49371
  • Gutenberg latest master branch at time of writing

Additional context

I was looking to create a PR to fix this on the block editor end. But as you can see in the inspector above, the margin rule that is being messed with already has pretty high specificity:

.wp-block-social-links .wp-social-link.wp-social-link.wp-social-link {
	margin: 4px 8px 4px 0;
}

On the flipside, the way editor styles work at the moment, they are effectively output like this:

<style>
[data-block] [data-block]:first-child {
	margin-top: 0;
}
[data-block] [data-block]:nth-last-child(2) {
	margin-bottom: 0;
}
</style>

That is, inline in the document, giving them extremely high specificity.

While an !important thrown in, or otherwise increased specificity would "solve" the issue, at the same time it's important the styles blocks are born with have as low specificity as we can possibly give them, so that themes can easily override them when they need to. So this should be fixed on the theme side.

I suspect the intent of the rules can be revisited entirely — I'm not quite sure what they are meant to address in the first place. Specifically the first-child and nth-last-child rules, these are very likely to affect any block with nesting, and notably horizontal blocks such as social links, navigation, buttons. But I also imagine basic things like group or columns would be affected by this.

If you can, I'd recommend simply removing the rules.

@carolinan
Copy link
Contributor

carolinan commented Nov 19, 2020

We had repeated issues with the positioning of both the old and the updated social links block.

#203

#716

This is a side effect of trying to keep up with block editor changes as they happen during the same release ☹️ without any sort of dev note or documentation of the changes.

And on top of that, any changes Gutenberg has made to this block on 9.3- current are not in 5.6 and the intention can not be that theme needs to have different styles for WordPress 5.3 - 5.6+ And different Gutenberg versions.

And on one side we are already at RC were making changes like this should be avoided.

@carolinan
Copy link
Contributor

With 859 merged, part of the problem is solved (postponed?) and this can be closed.

Related to the social links block only:
With Gutenberg 9.4.1 the block margins now overrides the top and bottom margins and the margin reset set by the theme.
But we can't remove the margin reset from the theme, because without the 9.4.1 the gap is very large.
I wish there was a better solution that did not leave code that is only used on some installs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants