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.

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

Cannot remove blockGap margin-top from full/wide blocks #37811

Closed
richtabor opened this issue Jan 10, 2022 · 8 comments · Fixed by #38613
Closed

Cannot remove blockGap margin-top from full/wide blocks #37811

richtabor opened this issue Jan 10, 2022 · 8 comments · Fixed by #38613
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@richtabor
Copy link
Member

richtabor commented Jan 10, 2022

Description

In the latest Gutenberg release, the blockGap value is now targeting the Group block's data-align="full"/`data-align="wide" div (which is added to full/wide blocks exclusively in the editor only). This means that an author cannot negate that block's margin-top (applied by the CSS snippet below), within the editor.

Note this affects any full/wide blocks, not just the Group block.

The CSS causing the issue:

.editor-styles-wrapper .edit-post-visual-editor__post-title-wrapper > * + *, .editor-styles-wrapper .block-editor-block-list__layout.is-root-container > * + * {
margin-top: var( --wp--style--block-gap );
}

This did work prior to the latest Gutenberg release, and there are use cases where margin should be negated (even if a theme has to add a class specifically to do so).

I think this should be resolved, even if the Group block receives margin controls (which I believe it should). I'd expect that the non-align wide/full blocks should perform the same as the others in this regard.

Step-by-step reproduction instructions

  1. Add two Group blocks, fullwidth to a page, assign a background color so you can see it
  2. Add a utility class, like .m-0 { margin: 0 !important; } to a theme stylesheet
  3. Add that class to the Group block
  4. See no effect within the editor, but negated margins on the front-end.

Screenshots, screen recording, code snippet

Screenshot of fullwidth group block in the editor, with margin unable to be modified:
CleanShot 2022-01-09 at 18 52 38@2x

Screenshot of fullwidth group block on the frontend, with classes appropriately negating blockGap:
CleanShot 2022-01-09 at 18 57 02@2x

Same class negating the blockGap value with non wide, or full, group blocks:
CleanShot 2022-01-09 at 18 54 56@2x

Environment info

Gutenberg dev

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

@richtabor
Copy link
Member Author

@youknowriad, @ndiego tagging you two as I know you've worked on Block Gap a good bit 🤜 🤛

@youknowriad
Copy link
Contributor

youknowriad commented Jan 10, 2022

Tested this and while the bug exists, I'm almost certain it has always existed, maybe it's just made visible by the block gap style (while previously no style was being applied to block wrappers).

The reason is simple, historically the editor adds extra divs to apply alignments in the editor and the margin is applied on the inner div and not the added wrapper div. I think that extra div might not be needed anymore with layout support and FSE themes but is required for legacy themes, that said removing it might not be that easy (backward compatibility...) but worth a try.

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Jan 10, 2022
@chthonic-ds
Copy link
Contributor

Tested this and while the bug exists, I'm almost certain it has always existed

Yes, here's an earlier version of this issue: #33142

@richtabor
Copy link
Member Author

I'm almost certain it has always existed

Yea you two are right, it was there prior to 12.3, I'm not sure how I was able to remove it previously — perhaps this was introduced with the full/wide blocks received the extra div?

@richtabor
Copy link
Member Author

The reason is simple, historically the editor adds extra divs to apply alignments in the editor and the margin is applied on the inner div and not the added wrapper div. I think that extra div might not be needed anymore with layout support and FSE themes but is required for legacy themes, that said removing it might not be that easy (backward compatibility...) but worth a try.

Legacy theme's don't support blockGap though, right? Perhaps if blockGap is supported, those extra divs are not needed? A tough problem for sure.

@youknowriad
Copy link
Contributor

I think what we should try is:

  • if layout is supported (theme.json file exists), which means alignments are controlled (and not provided by the theme), we should remove these divs.

I'm just unsure about the potential impact of such change.

@timsinc
Copy link

timsinc commented Jan 21, 2022

Related to this I've noticed that when block gap is defined and you group the first element of the root container, a style tag is generated and inserted at the top of the root container, which then causes the > * + * top margin block gap rule to be applied to the newly created group, which shouldn't be the case.

@giannis-koulouris-bb
Copy link

Also is it possible to not generate that CSS if I have opted-out from blockGap via theme.json? .wp-container-61f11256b17eb > * + * { margin-top: var( --wp--style--block-gap ); margin-bottom: 0; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants