Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Collapsed margins is breaking the reusable blocks border #9730

Closed
youknowriad opened this issue Sep 10, 2018 · 4 comments · Fixed by #9735
Closed

Collapsed margins is breaking the reusable blocks border #9730

youknowriad opened this issue Sep 10, 2018 · 4 comments · Fixed by #9735
Labels
[Type] Bug An existing feature does not function as intended

Comments

@youknowriad
Copy link
Contributor

Use two sequential reusable blocks in a post and notice that their borders are not spaced properly. It's probably a regression of the margin-collapsing PR

screen shot 2018-09-10 at 09 08 04

cc @jasmussen
@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Sep 10, 2018
@jasmussen
Copy link
Contributor

Are there any additional steps? I can't reproduce:

screen shot 2018-09-10 at 11 26 47

Note that although collapsing margins could definitely cause this, until such a time as we decide to move forward with #8350, that PR should not have affected margins. Yes, it enabled margin collapsing, but it also compensated for those collapsing margins to ensure there wasn't any visual change. "Why then?" you might ask? Well because now we can allow margins to collapse on a per-block basis, which is a useful tool for themers who want their editor styles to look as closely as possible to the end result.

@youknowriad
Copy link
Contributor Author

youknowriad commented Sep 10, 2018 •
edited
Loading

I guess it is surfaced in #9732

I thought it was a generic error happening for every reusable block but it only happens for reusable blocks with InnerBlocks usage (columns or multi-selection)

@jasmussen
Copy link
Contributor

I found out why this is. We apply negative margins to any block that contains a editor-block-list__layout element inside. I think we did this for columns, but I'll be tracking this down.

jasmussen pushed a commit that referenced this issue Sep 10, 2018
Adjust margin rules for nested blocks.
c9465f3 
This PR one one hand fixes #9730, and on the other hand simplifies some of the column wrangling code to be simpler.

It does visualy regress it slightly: previously we'd use negative margins to make it so text inside columns looked to be spaced the same as text before and after the columns.

However this greatly complexified the CSS, which caused #9730 in the first place. The thing is — the columns block itself has a margin. This margin collapses correctly to adjacent blocks. But given it also has children, those margins don't also collapse. Possibly we could make this happen, I'm unsure, flex containers are supposed to prevent margin collapsing by creating their own contexts, but it seems like the negative margins added complexity where it wasn't helpful.
@jasmussen jasmussen mentioned this issue Sep 10, 2018
Merged
@jasmussen
Copy link
Contributor

Pushed a fix. That will fix it for #9732 as well, but good to have separate for now.

jasmussen added a commit that referenced this issue Sep 10, 2018
@jasmussen
Adjust margin rules for nested blocks. (#9735)
aa39621 
This PR one one hand fixes #9730, and on the other hand simplifies some of the column wrangling code to be simpler.

It does visualy regress it slightly: previously we'd use negative margins to make it so text inside columns looked to be spaced the same as text before and after the columns.

However this greatly complexified the CSS, which caused #9730 in the first place. The thing is — the columns block itself has a margin. This margin collapses correctly to adjacent blocks. But given it also has children, those margins don't also collapse. Possibly we could make this happen, I'm unsure, flex containers are supposed to prevent margin collapsing by creating their own contexts, but it seems like the negative margins added complexity where it wasn't helpful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adjust margin rules for nested blocks. WordPress/gutenberg
2 participants
@youknowriad @jasmussen