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

Gutenberg Blocks should be inside divs #5292

Closed
saltnpixels opened this issue Feb 27, 2018 · 35 comments
Closed

Gutenberg Blocks should be inside divs #5292

saltnpixels opened this issue Feb 27, 2018 · 35 comments
Labels
[Type] Question Questions about the design or development of the editor.

Comments

@saltnpixels
Copy link

saltnpixels commented Feb 27, 2018

I posted an issue before but it was closed. But the problem has grown more, as I realized through playing around the issues that can come up.

Issue Overview

A lot of the blocks are output without being surrounded by any div or section element which makes it hard for styling and keeping content looking right.
I will try and explain some of the blocks with these issues.

Full Width Images

This is the big issue which leads to more issues.
It starts with wanting to have a centered, main column for your article with a max-width, yet accommodate both wide and full images.
If you want to have class .alignfull working, there are different methods. Some involve stretching the div.alignfull outside the centered content column with vw units.
That can be seen here:
https://themeshaper.com/2018/02/15/styling-themes-for-gutenberg/
It works, but this wont work on sites with a sidebar and also gives horizontal scrollbars.

Better is giving all the items directly inside the column, a fixed width, except the .alignfull and .alignwide items. This way ALL items are centered with a max-width except two items which go full width. The column iteself has no explicit width.

We will call this layout "Centered-Items" for the rest of this issue as opposed to Centered Column.
See here: https://codepen.io/saltnpixels/pen/JpwjOp

This layout is great, but because some items are not wrapped in a div they can look strange. Here are some.

Floated Images

A floated image, outputs a figure element with an inline max-width attached, along with a class of .alignleft.
In the Centered-Items layout if we floated the figure it would go all the way to the edge of the page because the column has no explicit width. What we can do instead is float whats inside the figure.
You need to use !important to override the max-width thats inline, so that the figure element matches your columns width. And a lot more styling is required for the caption and the image as they now are now not treated as one unit.
See the floated images in the link above. Note how ".alignright img" is floated now instead of just ".alignright".

If the figure was contained in a div we would not be having any of these issues.

Latest Posts Block

This outputs a naked unordered list as ul.wp-block-latest-posts
It can be found in the link above at the bottom of that page.
Continuing from the last example of a Centered Items layout. All the items directly inside the column would all probably need padding so the text doesn't bump up against the edge of the browser on mobile. If you view the link above and go mobile, you can see there is padding on the elements (except full width elements).
This ruins the ul's padding it would have otherwise had.
If the ul was contained in a div this would not be an issue. the ul would have its own padding, while the outer div would have the padding necessary for mobile or whatever needed. See image below.

(Padding for ul is messed up. ul should be inside a div. Outer div should have padding, not ul.)

screenshot 2018-02-27 15 39 43

there are probably other elements that need special padding and what not, but because they are not contained in a div.block they wont be able to have proper paddings. Like the Pre element.

Pre

pre elements with a background color are too big. They should be contained in a div.
The images work because the figure holds the image. Here nothing is holding the pre element.

Paragraphs

A paragraph block has no classes at all. It cannot easily be styled. you might want to style all p.wp-block-paragraph, as opposed to other p tags that appear in the article.

Conclusion

The rabbit hole would keep going with different elements and each one would need special styling.
If they would be contained in divs with wp-block classes, they would be real blocks and whats inside can be contained and styled properly.

@sc0ttkclark
Copy link

Related #4883

@jasmussen jasmussen added [Type] Question Questions about the design or development of the editor. Needs Design Feedback Needs general design feedback. labels Feb 28, 2018
@jasmussen
Copy link
Contributor

Also related, #5209 (comment).

I have similar thoughts to you, insofar as there is value in avoiding inline styles whenever we can. But I would like very much to avoid extra divs if at all possible. It's such nice markup when the image is simply inside a figure. As such, I've been exploring ways to accommodate wide and fullwide designs in ways that work with the existing markup. https://codepen.io/joen/pen/oEYVXB, as I've mentioned, is one approach.

Another suggested approach has been to make floated images become inline blocks. @mtias has some thoughts on how this might work.

@mtias mtias added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Feb 28, 2018
@saltnpixels
Copy link
Author

While it is nice to have less markup, (trust me, I hate tons of divs!), too little markup will leave no room for css control.
You have to work really hard to make the css work. One door will close and another will open.

I'm not asking for millions of divs. Just one. One div or section element surrounding each block. Otherwise you will need to play around for every single block case.
Have you tried your method with a pre block? There are more issues. Same with a latest post block.
Because on mobile, the items will need padding.
See my version of your pen on mobile:
https://codepen.io/saltnpixels/pen/JpwjOp

This image shows how we have padding on left and right of all blocks. Works nicely on paragraph blocks and images. See what happens to pre and ul blocks because they are not surrounded. So we have alignfull and alignwide working nicely, but now other blocks are not working nicely.

screenshot 2018-02-28 09 50 55

I like your method a lot. But we would need to surround each block to make it feasible.

@vfontjr
Copy link

vfontjr commented Mar 5, 2018

Gutenberg images are wrapped in a figure tag, which you can use just like a div. If you want the figure tag wrapped in an additional div, you can edit the HTML and make it anything you want it to be with the Gutenberg code editor. You can also copy the Gutenberg image block and make your own block that either includes the extra div or replaces the figure block. Personally, I don't want an extra div. Everything functions perfectly the way it is, and I've had no issues whatsoever amending the CSS. I think the general product should remain as light and clean as possible. The flexibility built into Gutenberg is substantial. Once developers grasp how to use the abstraction layers and discover how easy it is to create your own custom blocks, I predict there are going to be a lot of third party block libraries hitting the market.

@saltnpixels
Copy link
Author

@vfontjr

Gutenberg images are wrapped in a figure tag, which you can use just like a div.

Yes, but with the ability to go full width, things become a little harder to style.
The figure element is given the .alignleft or .alignright class along with a max-width. This makes it difficult to use as a div.
Furthermore other elements without a wrapper of any kind become difficult to style, like the items listed above in my first post.
Figure is somewhat easy compared to ul and pre elements which are not divs.

@vfontjr
Copy link

vfontjr commented Mar 5, 2018

So how is this different from the way WordPress works today? With today's WordPress, you still have to manually add a div, it automatically inserts the alignment classes, etc. In fact, if you dig into Gutenberg's code, the alignment bar is TinyMCE's.

@saltnpixels
Copy link
Author

Well now there is a figure class. Before there was not.

I found a fix.
The way i have fixed this for now is by adding padding around the whole column instead of the items inside. So the items each have a max-width, but not padding. That is still; reserved for the div holding all the content.
Then for .alignfull, I used negative margins equal to the padding.

@saltnpixels
Copy link
Author

saltnpixels commented Mar 5, 2018

I got it working for now using negative margins on .alignfull, while still giving the column no max-width.
It works well i think. Check it out on mobile and desktop.

https://codepen.io/saltnpixels/pen/JpwjOp

Still requires overriding the figure with !important
And make sure the .alignfull has no width or set it to auto because 100% ruins the negative margins.

@jasmussen
Copy link
Contributor

Glad you found a solution, @saltnpixels.

The solution you linked doesn't appear to change the way floats behave though, which in my understanding was the crux of this issue, correct? You still have to float both the image and the captions inside the figure, as opposed to floating the figure itself no?

Not saying that's a bad solution, it's the one I use myself.

@saltnpixels
Copy link
Author

@jasmussen
Yes floats are still an issue.

@afercia
Copy link
Contributor

afercia commented Apr 18, 2018

Not sure why this issue has the accessibility label, but maybe I'm missing something. /Cc @mtias

@mtias
Copy link
Member

mtias commented Jul 12, 2018

@afercia sorry, I worry wrapping markup in divs for styling purposes is not a sound semantic approach that complicates the user's content and I'd rather any shortcoming be solved on a per block basis with an eye towards their inherent semantics.

@saltnpixels
Copy link
Author

@mtias but having no markup makes it really hard to style or manipulate in any way.
We have to jump through hoops getting floats to work right and figuring ways out to do what would otherwise be simple.

And case by case means the markup will differ for each block.
You don't think there should be a standard markup for each block?

@afercia
Copy link
Contributor

afercia commented Jul 12, 2018

@mtias thanks for your feedback. I totally agree (as an old-school guy) that avoiding divities is a good practice. In this specific case though I don't see it strictly related to semantics, as the proposed use of a wrapper div is not a replacement for more semantic elements. It's just a wrapper with no semantics and will be ignored by assistive technologies. Will certainly leave the decision about styling and markup patterns to you and the team, just asked because we'd like to remove the Accessibility label to keep the report cleaner. Going to remove it, thanks!

@afercia afercia removed the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 12, 2018
@jasmussen
Copy link
Contributor

Noting that I'm working on aspects in #8350 and #7721.

Also figured out that you can use attribute selectors to emulate a disparete wp-block class, like so:

[class*="wp-block-"] {
  border: 10px solid coral;
}

See also https://codepen.io/joen/pen/djeobN

@webmandesign
Copy link
Contributor

Hi,

I've just written an article studying full-width alignment and applying a helper wrapper div around the aligned blocks. You can check it at https://codepen.io/webmandesign/post/gutenberg-full-width-alignment-in-wordpress-themes

Especially in the "The .align-wrap wrapper" section of the article you will find a helpful PHP function that can provide you with such wrapper div if needed.

Regards,

Oliver

@jasmussen
Copy link
Contributor

Hi @webmandesign, please see #7721 and #9189, which implements this wrapper when images are floated. This is already released in 3.6 and is usable today using this method: https://codepen.io/joen/pen/zLWvrW

Does that improve things?

Also, @saltnpixels can this issue be closed as a result?

@webmandesign
Copy link
Contributor

@jasmussen Thanks for pointing this out! I have shared my thoughts on #9189 but I will reply here too (instead of #7721):

For some reason, the new image markup doesn't work with me in Gutenberg 3.6.2. Here is what I got on front-end:

<figure class="wp-block-image alignleft is-resized">
	<img ...>
	<figcaption>Left aligned image with caption</figcaption>
</figure>

So, is this new markup integrated into 3.6+ version?

@webmandesign
Copy link
Contributor

webmandesign commented Aug 22, 2018

Sorry, now I've noticed that #7721 was merged 2 days ago, while Gutenberg 3.6.2 was released 5 days ago. So, the new markup is not included with 3.6.2 yet and that's why it's not working for me ;)

@jasmussen
Copy link
Contributor

Oh that's right, my apologies. The new markup slipped from 3.6 to 3.7, but it's in master if you want to try it.

@saltnpixels
Copy link
Author

@jasmussen
Pretty cool!

That would make it somewhat better. Would be nice if there was a class on the figure element that let you know there was a caption inside. That way we can style it when it has the caption and when it doesn't.

Lastly, ul and ol elements need to be wrapped. They have their own special margins and you can't use the margin auto on them easily...

I have created a new starter theme that implements Gutenberg and is really awesome. It's all set to use alignfull and alignwide, works with sidebars, includes a nifty app-like menu and uses css grid.

I am using Gutenberg for the documentation section. It's almost done.

https://ignition.press
Check it out!

@jasmussen
Copy link
Contributor

I'll leave this ticket open for now, as the PR I made "only" wraps floated images. I'm not personally convinced that every block needs div wrappers, I'd like to avoid them if we can to keep the markup as semantic as possible.

@Soean
Copy link
Member

Soean commented Aug 28, 2018

Related #9411

@simlawr
Copy link

simlawr commented Sep 9, 2018

I don’t understand the aversion to the pragmatic use of divs as a presentational hooks for styling. It’s not always possible to achieve certain styling effects without additional elements.

Surely the priority should be to provide the most flexible HTML possible?

There seams to be no semantic, accessibility or performance reasons why blocks could not be wrapped in divs, or at least give theme developers the option to control wrapping via a filter.

Indeed, I would like Gutenberg to give theme developers total control of block HTML.

@mtias
Copy link
Member

mtias commented Oct 7, 2018

@saltnpixels ignition.press is looking good!

Surely the priority should be to provide the most flexible HTML possible?

It's to provide the most semantically meaningful description of the document, for which divs are not adding anything valuable.

We should not transform the content of millions of users that way just because it adds more styling options. There will be container blocks for doing these things that are more explicit, and we should keep looking at semantic expressions (like figure around an image tag) to better express these units. Pullquotes are also a good example of a recent tweak in that direction.

Thank you all for the conversation here, I think it has raised some good points, and I'm feeling more inclined to start adding wp-block classes to more root elements, including a generic one. Going to close as we don't plan to add divs around blocks for now.

@stljeff1
Copy link

stljeff1 commented Mar 6, 2019

Maybe not a DIV, but it may make sense to use a SECTION tag in some cases. At the very least, whats the harm in providing a filter should the developer want the CHOICE to wrap block output? IE:

function my_block_filter($block_name, $block_content) {
    return sprintf( '<section class="block-%s">%s</section>', $block_name, $block_content);
}

add_filter('the_block_content', 'my_block_filter');

@stljeff1
Copy link

stljeff1 commented Mar 6, 2019

To anyone landing here (like me) looking to wrap block content in tags, I found the filter to do that. here is an example:

function test_filter($block_content, $block) {
     $block_name = get_html_friendly_name($block->blockName); 
     return sprintf( '<section class="block-%s">%s</section>', $block_name, $block_content);
}

add_filter('render_block', 'test_filter', 10, 3);

FYI: get_html_friendly_name is some arbitrary function that, in theory, will take the block's name and return a string that I can use as a class or ID attribute

@webmandesign
Copy link
Contributor

webmandesign commented Mar 7, 2019

@stljeff1 Great catch, thank you!
Just a minor update and fixes to your code:

function test_filter( $block_content, $block ) {
	if ( empty( trim( $block_content ) ) ) {
		return $block_content;
	}

	return sprintf(
		'<section class="block-%1$s">%2$s</section>',
		sanitize_title( $block['blockName'] ),
		$block_content
	);
}
add_filter( 'render_block', 'test_filter', 10, 2 );

You can use sanitize_title( basename( $block['blockName'] ) ) instead of sanitize_title( $block['blockName'] ) if needed.

@ThesisCreative
Copy link

ThesisCreative commented Mar 19, 2019

@webmandesign This is great solution. However, this is rendering a duplicate section tag for me. It renders like this:

<!--Correct section rendered -->
<section class="block-paragraph">
      <p>This is a gutenburg block </p>
</section>

<!-- But then also a second blank section rendered --> 
<section class="block-">

</section>

It creates this blank section for every new block I add in the WP Editor.

I can deal with this by just targeting and hiding the extra with CSS, but I'm sure there is an easy solution to remove the extra markup?

@webmandesign
Copy link
Contributor

@ThesisCreative You're right, thanks for spotting this. I've updated the code to prevent such issue.

It looks like WordPress outputs some empty characters (possibly just linebreaks) in render_block after rendering each block.

@ThesisCreative
Copy link

@webmandesign brilliant. Thanks so much for helping out. I'm primarily front-end so this solution was huge.

@ThesisCreative
Copy link

@webmandesign Noticing that this also adds sections to nested blocks, which can result in some bad syntax with sections inside sections, and prevents things like CORE Block columns from working properly (the extra element prevents the CSS flexbox styles from being applied to children of wp-bloc- column)

@webmandesign
Copy link
Contributor

webmandesign commented Mar 19, 2019

@ThesisCreative Please note that I only provided a basic simple solution :) I personally don't actually recommend wrapping every block in additional section/div tag. Personally I only use it for wide and full aligned blocks. For your specific usecase please adapt the function to your needs (using the $block variable).

@stljeff1
Copy link

Yes I concur with @webmandesign ... after I figured out how to wrap blocks, I quickly realized it could get out of hand easily, which I think was the essential argument made by WP... I use this functionality carefully on specific post types and/or blocks.

@nicmare
Copy link

nicmare commented Nov 4, 2019

@stljeff1 Great catch, thank you!
Just a minor update and fixes to your code:

function test_filter( $block_content, $block ) {
	if ( empty( trim( $block_content ) ) ) {
		return $block_content;
	}

	return sprintf(
		'<section class="block-%1$s">%2$s</section>',
		sanitize_title( $block['blockName'] ),
		$block_content
	);
}
add_filter( 'render_block', 'test_filter', 10, 2 );

You can use sanitize_title( basename( $block['blockName'] ) ) instead of sanitize_title( $block['blockName'] ) if needed.

if you have for instance a column block, it applies the wrapping div to the parent and child elements. is there any code to just wrap the very outer block and not the nested child elements?!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

No branches or pull requests