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

Remove block alignment wrapper in editor #33142

Closed
keithdevon opened this issue Jul 1, 2021 · 17 comments · Fixed by #38613
Closed

Remove block alignment wrapper in editor #33142

keithdevon opened this issue Jul 1, 2021 · 17 comments · Fixed by #38613
Assignees
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Needs Decision Needs a decision to be actionable or relevant Needs Technical Feedback Needs testing from a developer perspective. [Status] In Progress Tracking issues with work in progress

Comments

@keithdevon
Copy link

What problem does this address?

In the editor, blocks with alignment (e.g. Wide, Full) are wrapped in a div with the .wp-block class and a data-align attribute.

All other block specific classes are then added to a child of this block.

<div class="wp-block" data-align="full">
    <div style="background-color: rgb(185, 204, 70);" class="wp-block-group has-brand-green-background-color has-background" ...>...</div>
</div>

This means that selectors such as .wp-block-group.alignfull.has-background have no effect in the editor.

This leads to two problems (that I know of):

  1. Frontend theme styles often break and have to be recreated with specific editor stylesheets.
  2. Sometimes, useful selector rules, particularly nth-of-type, nth-child, and adjacent operators, cannot be used in the editor.

For example, if you want to remove margins between two full-width groups with background colours you might do something like this:

.wp-block-group.alignfull.has-background + .wp-block-group.alignfull.has-background {
    margin-top: -50px;
}

This has no effect in the editor as there is no elelment that can contain all of these classes - they are split onto two levels of div.

Do we need the wrapper with .wp-block and the data-align? Blocks with no alignment don't have a wrapper, so I'm assuming it isn't used as a hook for the editor UI.

What is your proposed solution?

Remove the <div class="wp-block" data-align="full"> div and add the wp-block and alignment classes to the block level element.

E.g. <div class="wp-block wp-block-group has-background alignfull ..." ...>...</div>

If needed, the data-align attribute can be added in addition to the alignment class.

E.g. <div class="wp-block wp-block-group has-background alignfull ..." data-align="full" ...>...</div>

@justintadlock
Copy link
Contributor

justintadlock commented Jul 2, 2021

Wait until you start fighting <style> (group inherit layout styles) and <svg> (duotone) tags stuck between blocks too. The following is just covering something like the 80% - 90% of cases I've seen. I'm not even sure if it can be fixed, but removing the [data-align] wrappers would sure help.

.is-root-container > :first-child,
.is-root-container > style:first-child + [class*=wp-block],
.is-root-container > [data-align]:first-child > [class*=wp-block]:first-child,
.is-root-container > .block-editor-block-list__block:first-child > :first-child,
.is-root-container > style:first-child + [data-align] > :first-child,
.is-root-container > [style*=hidden]:first-child + style + [data-align] > :first-child,
.wp-block-cover__inner-container > style:first-child + [class*=wp-block],
.wp-block-template-part > style:first-child + [class*=wp-block],
.wp-block-embed > .wp-block-embed,
.wp-block-spacer + [data-align] > :first-child,
.wp-block-group > [data-align]:first-child > :first-child,
.wp-block-group > style:first-child + .wp-block-group,
.wp-block-group > style:first-child + [data-align] > :first-child,
[data-align=full] + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] > [class*=wp-block]:first-child > [data-align=full]:first-child > [class*=wp-block]:first-child,
[data-align=full] + [style*=hidden] + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] + [style*=hidden] + style + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] + style + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] + style + style + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] + style + [style*=hidden] + [data-align=full] > [class*=wp-block]:first-child,
[data-align=full] + style + [style*=hidden] + style + [style*=hidden] + [data-align=full] > [class*=wp-block]:first-child {
	margin-top: 0;
}

@richtabor
Copy link
Member

After testing theme.json themes today, I noticed this wasn't an issue (though I couldn’t really nail it down). Are the standard block margins removed if a theme has a theme.json file?

@justintadlock
Copy link
Contributor

After testing theme.json themes today, I noticed this wasn't an issue (though I couldn’t really nail it down). Are the standard block margins removed if a theme has a theme.json file?

The [data-align] wrapper is there for me regardless of whether I am using a theme with a theme.json.

I'm not sure what the standard block margins are. Margins for me are added by the theme.

@richtabor
Copy link
Member

@justintadlock The wrapper is still there, but the default block margins are removed from themes with theme.json (per this PR #30375 I found). That's great for theme.json themes, but not for any current themes. :/

@justintadlock
Copy link
Contributor

The source of default block margins doesn't really matter. As long as the [data-align] wrapper is there, it's pretty much impossible to correctly use sibling selectors.

For example, you can't kill margins for this:

<div data-align="full">
	<div class="wp-block-example"></div>
</div>


<div data-align="full">
	<div class="wp-block-example"></div>
</div>

With CSS like:

.wp-block-example {
	margin-top: 2rem;
}

.wp-block-example.alignfull + .wp-block-example.alignfull {
	margin-top: 0;
}

That CSS works on the front end because the HTML is this:

<div class="wp-block-example alignfull"></div>
<div class="wp-block-example alignfull"></div>

You can certainly do some weird and hacky workarounds to sort of make it work in the editor...until it doesn't. And, this is just the most basic of examples.

:first-child, :last-child, and :nth* often get squashed by this too.

And, it's not all about margins. It can be any style rule -- for example, a theme author might want to add/remove bottom/top borders between siblings, alternate default background colors, or a million other things.

@justintadlock
Copy link
Contributor

Just to show a slightly more complex example, when multiple File blocks appear after one another, I want them to look like this:

stacked-file-blocks

What I've done there is target .wp-block-file + .wp-block-file to remove the top margin and border from the sibling blocks. This is easy to do on the front end.

However, if I add a "wide" alignment (or "full") to the File blocks, the [data-align] wrapper makes it impossible to target sibling File blocks. And, I see this in the editor:

editor-wide-file-blocks

There is a workaround for this very specific scenario, which is to add them all to the Group block without layout inheritance.

@richtabor
Copy link
Member

The source of default block margins doesn't really matter.

I'm saying the default 28px block margin does not exist in theme.json based themes; there wouldn't be a need to zero them out. But I do agree that being able to target specifically is useful. I'm wondering though if this control is moving more towards a "user-centric" model, where a block's margin is controlled via a spacing control — not CSS.

@richtabor
Copy link
Member

However, if I add a "wide" alignment (or "full") to the File blocks, the [data-align] wrapper makes it impossible to target sibling File blocks. And, I see this in the editor.

I've found targeting the > sibling, then adding a -28px margin top/bottom work — but it's janky. 🤨

@justintadlock
Copy link
Contributor

I'm saying the default 28px block margin does not exist in theme.json based themes; there wouldn't be a need to zero them out.

There would be because themes must still use margin, padding, or some other method to handle whitespace between elements.

But, this ticket is not about the existence/non-existence of core default margins or even specifically about margins.

It is about inconsistent markup between the front end and the editor. The editor markup is causing a situation where it is impossible to target sibling blocks of the same type (and related issues with :first-*, :last-, and :nth- selectors).

Canceling margins is just an example and common use case.

I'm wondering though if this control is moving more towards a "user-centric" model, where a block's margin is controlled via a spacing control — not CSS.

Users should never have to micro-manage every block's spacing (at least those with wide/full alignment). They should have the choice, but themes need to be able to design a solid foundation and set up smart defaults for spacing and everything else (again, this ticket is not specifically about margins).

I've found targeting the > sibling, then adding a -28px margin top/bottom work — but it's janky. 🤨

The > symbol targets a direct child and not a sibling. It's useful, just not for the provided example.

@richtabor
Copy link
Member

@justintadlock I'm not discounting the need/issue, just providing context on how themes.json based themes are handling this.

I'm finding more and more that the transition between pre and post theme.json themes will be a bit finicky for a while as we head further and further down this trail.

@keithdevon
Copy link
Author

After a bit of digging I found the original PR that introduced the new alignment wrappers.

In the PR, @ellatrix states that:

It doesn't really belong in this component, which is meant to render a single element.

In the code itslef there is a comment that states:

For aligned blocks, provide a wrapper element so the block can be positioned relative to the block column.

My questions are:

  1. Why do need an alignment wrapper at all.
  2. What is the 'block column'? Is this still a concept that we use?

@skorasaurus skorasaurus added Needs Decision Needs a decision to be actionable or relevant Needs Technical Feedback Needs testing from a developer perspective. CSS Styling Related to editor and front end styles, CSS-specific issues. labels Jul 28, 2021
@chthonic-ds
Copy link
Contributor

Seeing a decision made on this soon would be wonderful, if at all possible. I imagine a number of block-based/FSE themes are on the boil right now and not having to account for the [data-align] wrapper will mean not having to commit to some tricksy editor-specific CSS, while also surfacing some sibling .has-background relationships that are currently difficult (impossible?) to target.

This comment #23089 (comment) in the linked PR suggests it's possible to remove the [data-align] wrapper. Is it possible to handle this in a similar way to how the Group block deprecated the inner container i.e. the wrapper is generated only for non-layout supported themes?

@willrowe
Copy link

It would be great if the [data-align] wrapper could be removed. It really throws a wrench into displaying things correctly in the editor. In some cases for custom block types it even makes them disappear due to the unexpected styles applied to [data-align].

I would expect the same exact classes (alignleft, aligncenter, etc) to be applied to the block wrapper, not for the whole block to suddenly be wrapped in div with a data attribute.

@ZebulanStanphill
Copy link
Member

I, too, have run into some trouble lately with the alignment wrappers. It seems like nearly every theme I've tried has trouble with styling the block editor... especially when it comes to alignments. One of the most popular (and frequently updated) themes right now, Astra, currently has tons of broken editor styles including alignments, and frankly, it's hard to blame them. Trying to support the block editor right now is a lot of extra work for themes right now, because it's a confusing mess of inconsistent markup and specificity hell.

It would really be nice to see this get prioritized. The WYSIWYG-ness of the editor is the whole point, isn't it? All the major page builders have done a much better job at this, and while I understand why Gutenberg hasn't gotten there yet, it's nevertheless disappointing to see editor styling in its current state. It makes the editor feel broken and unfinished, despite all the wonderful improvements made since WP 5.0.

I'm guessing the work on moving the edited content to an <iframe> is the main thing holding this up. Is that correct, @ellatrix? I know you've been working a lot on that lately, so thanks for that.

@fklein-lu
Copy link
Contributor

@ZebulanStanphill If it helps, #33142 describes a workaround for the current alignment issues in hybrid themes.

@Humanify-nl
Copy link

Humanify-nl commented Oct 31, 2021

To add to @ZebulanStanphill's comment .

I'm relatively new to Gutenberg development, and one of my biggest challenges has been making sense of the editor HTML. I'm not sure i'd call it broken (i'd say its unfinished), but it definitely raises some eyebrows.

When I stepped into Gutenberg, it promised this amazing sync between back and front end, which is there for the user, but the reality for a developer is different. We have to maintain 2 different stylesheets (and sometimes fight style.min.css 's opinionated styling).

Since this (alignwide support) is at an experimental state, I hope it gets attention.

@bethalessi
Copy link

This is a major problem for me too. It drives me crazy that I can't target the block. I wish :has() was able to be used. Instead I'm using margin hacks and it feels wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Needs Decision Needs a decision to be actionable or relevant Needs Technical Feedback Needs testing from a developer perspective. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.