Problem/Motivation

There is a desire to improve the markup of blocks, in particular, to allow the customization of which elements are used and to use section by default when there is a label.

Meta issue: #1980004: [meta] Creating Dream Markup
Issue based on: #1898034: block.module - Convert PHPTemplate templates to Twig

Proposed resolution

Make the template allow this customization.

Remaining tasks

Decide on markup. Implement.

User interface changes

Markup changes.

API changes

None.

Data model changes

None.

Release notes snippet

Issue fork drupal-1982244

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oresh’s picture

Project: » Drupal core
Version: » 8.x-dev
Component: Twig templates conversion (front-end branch) » markup
Category: feature » task

moving to core issues.

chippper’s picture

First off, +1 to everything you said, Oresh.

I'm not sure if this is the place to ask this, but could we do something about the classes themselves that get attached to blocks? Here is a sample stock block from a site I helped build:

class="block block-block block-21 block-block-21 even" id="block-block-21"

This has always struck me as utter madness. There is no reason on earth for that level of redundancy. Something like this makes so much more sense:

class="block block-21 even" id="block-21"

I suspect that the crazy classes were introduced to account for IE6, which had issues parsing multiple-class selectors (it didn't). But, as D8 no longer takes IE6 into account, we can simplify the class list, and can use multiple selectors in our CSS if need be. Using the example above, you could write CSS like:

.block { ... }
.block-21 { ... }
.block.even { ... }
.block-21.even { ... }

Sadly, it's probably far too late to bring up baking in functionality like the Block_Class module. It's not a huge deal, as hopefully it will still exist in contrib.

oresh’s picture

@chippper,
as far as I know, even/odd will not be added by drupal, cause now we can use css3 selectors :)
Secondly, i think there was an issue for dropping the block position number (block-21) both from class and ID, cause it doesn't fit so well in the new layout system, where the same block can be in different places for different pages.

I wish there were only .block .block-{{type}} classes added - it's just enough for us.
But anyhow - custom themes will be able to drop unneeded classes from it if needed )

chippper’s picture

@oresh

Thanks for the catch up. I didn't know that even/odd classes were getting dropped - all the better!

I wasn't sure how ID's were being determined - obviously, it's a machine-determined ID. I didn't realize it was due to position - I though it was simply in reference to the order in which the blocks were made.

Either way - my biggest issue was the crazy "block block-block block-" business. It's a block, leave it at that! If there are variations you want to put on it, leave the redundant block moniker:

class="block view machine-name-for-the-view-without-view-used-again"

or

class="block classname-determined-by-the-module"

etc.

mortendk’s picture

We dont wanna use the machine name for the block - cause it dosnt matter which module that created the specific piece of markup ;)
what would be really interesting is getting the right aria roles for the block's

i would prefer something like this and use the section class instead as a base

<section class="{{ attributes.class }} " {{ attributes }}>
  {{ title_prefix }}
  {% if label %}
    <h2{{ title_attributes }}>{{ label }}</h2>
  {% endif %}
  {{ title_suffix }}
  {{ content }}
</section>
ry5n’s picture

I want to second @chipper. Let’s not add all those useless `block-block-blockblockblock` classes. I’m tempted to suggest that blocks deserve no classes at all in core, but that might be extreme. We should probably go with @oresh: class="block block--{{type}}".

@mortendk

it dosnt matter which module… created the specific piece of markup

Yes! Hallelujah! :D

#5 looks good, except something about using <section> makes me worry. Even though http://html5doctor.com/the-section-element/ seems to suggest it would be OK (even ideal) the fact that blocks really are generic containers makes me nervous about using a sectioning element… Like what if my main nav is output by menu_block module, do I get a <nav> inside a <section>? That seems odd.

chippper’s picture

re: section vs. div - I second @ry5n - I think blocks should be divs. Blocks can be quite arbitrary in their purpose, so I just feel that div is a better piece of markup. That is, it would make sense for a section to contain several blocks. Also, as it is quite a common use case to *not* have a title on a block, we have a situation where a block won't have a header a good portion of the time. Per the HTML5 doctor - that's an indication that you don't want to use section.

I would propose this:

<div class="{{ attributes.class }} " {{ attributes }}>
  {{ title_prefix }}
  {% if label %}
    <h3{{ title_attributes }}>{{ label }}</h3>
  {% endif %}
  {{ title_suffix }}
  {{ content }}
</div>

I also changed the title to h3's, per the issue summary (which I agree with).

chippper’s picture

Status: Active » Needs review
FileSize
683 bytes

Created a patch for the markup above. Would love to hear any further thoughts.

Status: Needs review » Needs work

The last submitted patch, dream-markup-block-1982244-8.patch, failed testing.

chippper’s picture

I still kind of a n00b at core patches, so I can't make heads or tails of test failure. As near as I can tell, it fails making "block markup", which is kinda what is being changed :)

I re-rolled a patch, but all it does is remove the @todo in the comments pointing to http://drupal.org/node/1972122, as this patch removes the wrapper around the block content.

chippper’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, dream-markup-block-1982244-10.patch, failed testing.

ry5n’s picture

@chipper The markup in #10 looks good. Two things:
1. I’d like to hear from @mortendk about <section> before anything gets committed.
2. I see no rationale provided in the issue summary or the comments for h2 → h3.

Tests are failing because we removed the content wrapper, there are places where tests look for that. I’ve attached a patch that should fix the failing tests, and I’ve restored the heading to <h2>. I’m not opposed to that change, but we need a good reason for it.

ry5n’s picture

Status: Needs work » Needs review

And let’s see what the testbot says.

chippper’s picture

@ry5n - I'm open to arguments in favor of using section, I just thought given the wide number of use cases for blocks, div was a better fit. I think this will boil down to matters of opinion. I just picture a page on a site that uses a bunch of blocks in a sidebar, and feeling weird about a ton of section elements, as opposed to div's. Of course, that also might just because I've used div's for years. I guess I'm also thinking of situations where a group of blocks is presented together as related content, and div's making more sense in that situation.

re: h2 -> h3: True, a rationale wasn't provided, but it was questioned.

I feel that most people would be better served with blocks having h3's to start with as the default. *My* rationale for it is this - h2's (obviously) carry more of a semantic weight than h3's. However, in *my* experience, blocks tend to be used for secondary page content - like, say, in a sidebar. I would argue that the main content on the page - the 'body' area, if you will - should be allowed to have at least two levels of headers before semantically being on the same level as a block.

There is also the practical reality of SEO - search engines give some weight to h2's, while they don't really look at h3's. Most blocks hold secondary content that is not what I want a search engine to give weight to on a given page. For that reason, on most of *my* projects, I've had to override block.tpl.php to change h2's to h3's.

This is all derived from my personal, anecdotal, experience. I don't have any empirical evidence to back this up. I do realize that I'm painting in broad strokes here, and making a ton of assumptions. But, hey, this is *my dream markup*.

Given how easy this to override on at the theme level, it's obviously not a huge issue. I just was trying to save a step in what I perceive to the the most common use case for blocks.

I would love to hear what others have to say. Also, @ry5n, thank you for correcting the tests for the new block markup - I had no clue how to do that!

ry5n’s picture

@chippper Thanks for the rationale on h2 → h3. Good arguments. I’m fine either way, but it would be good to hear from others.

About <section>, I’d rather stick with <div> too: it seems safer to rely on block content for semantics; I was just wondering if @mortendk had some rationale I hadn’t thought of.

chippper’s picture

I took it upon myself to re-roll the patch from #13 agains the current HEAD, plus I switched h2 back to h3. Again, @ry5n, thank you for fixing the stuff the bot was testing against.

I'm just doing this now, so it doesn't fall through the cracks. If someone has strong opinions about the div/section or the h2/h3 questions, I'm still open to what they have to say.

Status: Needs review » Needs work

The last submitted patch, dream-markup-block-1982244-17.patch, failed testing.

chippper’s picture

Dag nabbit, - bear with me, I'm still figuring out how to change the tests for the bots. I figured out the second point of failure, but I still don't know what to do to correct the first. I'm just seeing what the bot does with this.

chippper’s picture

Status: Needs work » Needs review
NaheemSays’s picture

It is often the case where the block content may need separate/additional styling from the title.

Often the content may have different padding or a different styling to the title.

By removing the class around the content, anyone who wants to do this will need to modify the core block template.

While this is not a blocker as a theme having its own block implementation is not a bug but a feature, I think that it shouldn't be ignored when making the changes.

Status: Needs review » Needs work

The last submitted patch, dream-markup-block-1982244-19.patch, failed testing.

chippper’s picture

@nbz - what you say is true, that *sometimes* there is a need for different styling between the content of the block, and the title. I personally have *never* needed that extra div - but that is my own anecdotal experience. I would also argue, as a front end dev, if you need different spacing around the content than the title, you would be better served using margins being applied to whatever markup element is actually *in the content* (be it a ul, p - whatever).

One of the biggest fundamental shifts, from a front end perspective, in Drupal 8, is towards leanness of markup by default. To quote the meta issue:

Keep in mind that one of our principles is to start from nothing. That means using as few HTML elements and attributes as possible in core. If Bartik needs a div around something for its own purposes, that div tag should be added in the Bartik theme, not in core.

If we follow that principle (more on those here), then I would argue the place for the extra

is in the final theme - not in core by default.

Another way you could look at it is that we're trying to serve the 80% here - and I would say well over 80% of the time, you don't need that extra

around the content.

Also - a separate issue was created *just to remove that div*.

jgrubb’s picture

Hi, try this. h2 -> h3 based off of @ry5n's last patch (and HEAD as of today). For some reason @chippper's patch wouldn't work for me even though it looked identical - whitespace maybe?

star-szr’s picture

Status: Needs work » Needs review

Thanks @jgrubb! Sending to testbot.

markhalliwell’s picture

Status: Needs work » Needs review
+++ b/core/modules/block/templates/block.html.twig
@@ -39,16 +39,12 @@
-<div{{ attributes }}>
+<div class="{{ attributes.class }}"{{ attributes }}>

I'd really like to not change this. There's no real need to separate the classes from the attributes. This is generally only done if we're manually adding a class via the template (ie: like clearfix) or something. Since we're not, it should just be changed back.

+++ b/core/modules/block/templates/block.html.twig
@@ -39,16 +39,12 @@
   {{ title_prefix }}
   {% if label %}
...
+    <h3{{ title_attributes }}>{{ label }}</h3>
   {% endif %}
...
+  {{ content }}

The <h2/> to <h3/> change is fine. What I'm not too happy about is {{ title_attributes }} and {{ label }}. Per @joelpittet's request in #1903746-84: Replace the views grid table template with one using divs and what he's trying to do in #1968398: Convert Views $row_classes to $row['attributes'] , I have to agree with him that we should make arrays that are "drillable" here.

I really think this should be something like:

{{ title.prefix }}
  {% if title.content %}
    <h3{{ title.attributes }}>{{ title.content }}</h3>
  {% endif %}
{{ title.suffix }}
+++ b/core/modules/block/templates/block.html.twig
@@ -39,16 +39,12 @@
-  <div{{ content_attributes }}>
-    {{ content }}
-  </div>
+  {{ content }}

Not too happy about this. This helps target the content in blocks for themes. For instance:

.block .content ul li {
  background: #f00;
}

This would affect just the <li/> elements in the content, if we took out .content then we would be affecting all the <li/> elements in the block. This includes the contextual links printed in the title prefix/suffix.

markhalliwell’s picture

Status: Needs review » Needs work

oops, forgot to change status

mortendk’s picture

Status: Needs review » Needs work

Generelly as i look at this patch its cleanup the basemark & make it easy to figure out whats going on aka making stuff visibile :

<div class="{{ attributes.class }}"{{ attributes }}>
having class put explicit in is awesome i wanna kick in the class he needs no more preprocess bs ;)

{{ content.title }} is a better name though than "label" cause that makes no sense to me when i read it the first time im thinking <label>

{{ content }} im in favour of remoing the .content wrapper,

.block-fooo li{
  background: hotpink
}

Contextual links shoudnt be effected by this (didnt we fixed this way back in the html5 conversion) - so thats a bug in contextual links imho

chippper’s picture

@jgrubb - thanks for posting this. I don't know what I was doing wrong, I was just not able to modify the tests to accept my changes. I admit that is something I know very little about.

@Mark - you bring up several really good points. I'm of two minds on some of this, and it may come off a little contradictory:
- re: class="{{ attributes.class }}" - I actually *like* this as it makes the template more approachable to those new to Drupal (or those getting more and more familiar with the theming layer). Want to add a specific class to your template override? There you go. As morten points out - no preprocess stuff needed. The trends in modern front end development (e.g. SMACSS, OOCSS, et al) tend to lean heavily on classes for reusable elements. Anything we can do to help to make *that* simpler, is a good thing.
- re: label vs. content.title (or whatever) - I agree, a more intelligent variable/drillable array would be welcome. I also agree with morten that "label" is a terrible variable name.
- re: the .content div - as mentioned in #23 - one of the guiding principles for the Dream Markup is to 'start from nothing'. Is the .content div *sometimes* useful? Yes, I suppose so. I would venture to say the vast majority of the time it isn't. [I have yet to run into a project where I needed it.] Based on the goals of the initiative, I think we should err on the side of omission. It's easy enough to add back in with your own block override.

I'm aware that I'm arguing for more complexity on point 1, but arguing for simplicity on point 3. I'm just thinking of this from a (new to Drupal) front end dev's point of view - working with an existing class attribute is easy to see, touch, modify. So is adding a div with the class of 'content' if your theme needs it. Ripping apart {{attributes}} and modifying classes - that's a much more difficult thing to *learn* how to do, especially if terms like "array" sound foreign to you.

From my point of view, unless we come up with a better solution for what now is {{ label }}, the markup in the twig file in #24 looks good to me. When I applied it to my local install, the changes took effect nicely. I added a custom block, which seems to insist on a couple div's around the content (but this is true of HEAD as well).

markhalliwell’s picture

Had a talk with @mortendk about this in irc. Sorry for the confusion. I forget about the "start from nothing" aspect. I think removing .content is fine. But in the spirit of "starting from nothing", then we should also remove class="{{ attributes.class }}" too. We can add this "example of simple addition of classes" to Bartik. I'm no performance expert, but logic seems that having an extra Twig {{ variable.attribute }} just adds overhead because it needs to parse that variable. We don't need this for core or testing purposes.

As far as the variable and attributes names go, what if we did something as simple as:

<div{{ attributes }}>
  {{ title.prefix }}
  {% if title.content %}
    <h3{{ title.attributes }}>{{ title.content }}</h3>
  {% endif %}
  {{ title.suffix }}
  {{ content }}
</div>
star-szr’s picture

As far as class="{{ attributes.class }}" and performance, yes there is overhead and most of it is in the Attribute class. We profiled it as part of #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch, unfortunately I can't remember the specific issue that would show the difference.

mortendk’s picture

having all attributes collected is imho horrible for us in the fronend it dosnt make it easy to work with
this will explain to me really quickly what i need to do to get my work done

<div class="{{ attributes.class }}"{{ attributes }}>
  {{ title.prefix }}
  {% if title.content %}
    <h3{{ title.attributes }}>{{ title.content }}</h3>
  {% endif %}
  {{ title.suffix }}
  {{ content }}
</div>

it might just be a documentation issue though

markhalliwell’s picture

<div class="{{ attributes.class }}"{{ attributes }}> also has the issue of class="" getting printed if there are no classes. Edit: This wouldn't be an issue if it were something like <div class="clearfix {{ attributes.class }}"{{ attributes }}>, which would just print class="clearfix".

falcon03’s picture

Markup proposed in #30 looks really clean to me. But, why should we change the "h2" heading wrapping the block title to "h3"? From an accessibility point of view I won't recommend doing it.

Question from a novice: Using the proposed markup, could a theme add ARIA roles to the attributes twig variable of a block or modify the existing ones, if needed? If so, what would it be the most appropriate way to do it? A preprocess function inside mythemename.theme?

chippper’s picture

@Mark - you bring up a really good point about 'class=""' if there are no classes at all. But - this is Drupal we're talking about, here - when was the last time you saw a block with no class at all? I get that there is an overhead cost, though.

@mortendk - I like this proposal overall the most - it's still the most approachable, while still being clean.

If we -do- end up with just {{attributes}} - for the very good reasons Mark and Cottser point out - maybe we need to make sure in the commented area of the template we include instruction on how to pull the array apart, or to include the example of "class='{{ attributes.class }}'". I just have too many memories of reaching something in Drupal and going "WTF" and it taking hours of research to figure things out. Drupal doesn't always do a great job of explaining things with the context needed by a new user. I just want to save hours of frustration for future front end dev's :)

@falcon03 - *my* rationale for changing h2 -> h3 was in #15. It boils down to -I think- h2's for block titles are too 'heavy' semantically, and in the real world situations that I've been in, I've almost always had to change the h2 to an h3 for SEO reasons. This stems from the assumption that blocks are *most commonly* used for secondary content on a given page. I'm talking the ~80%~ use case. Are there situations where an h2 is appropriate? Sure - but I think *out of the gate* users would be better served if the default has h3.

markhalliwell’s picture

I just have too many memories of reaching something in Drupal and going "WTF" and it taking hours of research to figure things out. Drupal doesn't always do a great job of explaining things with the context needed by a new user. I just want to save hours of frustration for future front end dev's :)

I still don't like <div class="{{ attributes.class }}"{{ attributes }}> (personally) from a coding standpoint. We're not actually inserting any additional [static] classes....... but....... I do think it is really important that we help new Drupalers into the fold as best we can.

I guess I'll change my vote, we should probably keep it then. Besides, the issue @Cottser mentioned: #2006282: Refactor Attribute classes - Cleanup, Security, and Readability and minor performance will probably nick the performance issue to a negligible amount I think.

I agree about the <h2/> to <h3/> change. Yes, it's mostly from an SEO standpoint. Node titles in views[-blocks] should render a <h2/>, but this is about blocks. They are/can be everywhere and screw with SEO.

Ok, the patch is still NW because we still need to change the variables sent to the template so that the title variable has arguments instead of using separate underscored variables.

Tagging for an issue summary update.

falcon03’s picture

From an accessibility Point of View, blocks headings should be h2, not h3. As @chippper said, h2 is heavier semantically than h3. Let's think for instance to the main menu: an h3 is completely inappropriate for it. So, I'm against the h2->h3 heading change. Modifying headings would require re-thinking all the headings page structure; if we really want to do so (maybe using HTML 5 headers), let's discuss it in another issue and - most of all - not only for blocks, but thinking about the whole page. Screen reader users rely on headings very very much to understand what (and how) important something is.

markhalliwell’s picture

@falcon03: we are #2011578: Markup for Stark's page.html.twig + maintenance-page.html.twig

Instead debating all of this, can someone please reroll with the changes mentioned in #36. Thanks.

falcon03’s picture

@Mark Carver: I read up all the comments in #2011578: Markup for Stark's page.html.twig + maintenance-page.html.twig and there isn't any reference to headings. I like the markup cleanup going on in that issue, though. In any case the h2->h3 change shouldn't happen here.

Jeff Burnz’s picture

Arguing for h2 or h3 is really the wrong way to approach this - we have ask if they mean anything different - do they tell the agent anything different?

When I test h2 or h3 in an outline tool it tells me no, it does not matter, html5 outline algorithm does not care either way - it does not "skip" a branch, it just appears to think "this is lower ranked than the last heading level in the same section so I put it here in the tree". I am using the notion that ALL blocks have the same heading level.

If we use section element - even then it does not matter for most user agents regarding heading levels, because heading levels reset inside all sectioning elements, so again it does not really matter what you use for the first element, only that you sensibly structure heading levels thereon down.

Drupal page outline is seriously bad right now, but a pretty decent outline can be had if we use section element like morten suggested, but it takes conditional logic to really make it work and that afaict has been frowned upon in core templates like block, although surely Views raises some questions against this thinking.

Here is a small resoruce on what I am talking about, http://www.456bereastreet.com/archive/201205/make_sure_your_html5_docume...

[edit]ed for clarify.

derheap’s picture

1/ div vs section.
We should stick with div for now.
Block is also used as a wrapper on the main content.
Having a role="main" with only one section is useless.
Div is more adequate here.
On the other hand, the block for main should just return the content, no extra wrappers.

2/ h2 vs. h3
We should listen to the accessibility and stick with h2.
you can’t make one block default for all use cases, as blocks can be everywhere.
if sidebars are wrapped in section, than the html5 outline will be correct anyway.
No matter if you use h2 or h3.
if you want somthing special, fine just write a special block template for sidebars in your theme.

3/ {{ attributes.class }}
As we have a way to much classes added to the div, the case of an empty class="" would not happen that often.
An using {{ attributes.class }} make it clear to the themer where to add classes without hunting for a php-developer to add the classes via a preprocessor.
And as a themer, when I spot a empty class in a block, i will remove the attribute from the template.

<div class="{{ attributes.class }}"{{ attributes }}>
  {{ title.prefix }}
  {% if title.content %}
    <h2{{ title.attributes }}>{{ title.content }}</h2>
  {% endif %}
  {{ title.suffix }}
  {{ content }}
</div>

And if we ever manage to generate empty class tags (which we should aim at) we could use to following template:

{# If you want to add a static class to the template use the following <div>#}
{# <div class="{{ attributes.class }} YourClass"{{ attributes }}> #}
{# instead of the normal <div> on the following line: #}
<div{{ attributes }}>
  {{ title.prefix }}
  {% if title.content %}
    <h2{{ title.attributes }}>{{ title.content }}</h2>
  {% endif %}
  {{ title.suffix }}
  {{ content }}
</div>
martin_q’s picture

@Mark Carver
The variables title_attributes, title_prefix and title_suffix seem to be being set up by default in theme.inc. Are you saying we need to remove this everywhere? Thought I should ask before embarking on that!

I'm new to much of this but just trying to help in the sprint. So please ignore if I'm totally wide of the mark!

markhalliwell’s picture

@martin_q, yeah... that should probably be a separate issue (if one doesn't already exist, try searching first). We'll need to change all templates with this new issue.

@derheap, @mortendk:
Let's do the following. We need to keep stark (core) clean and simple to start out with. The code "example" should be at the top of the docblock, not above the code (for api.d.o). Since @martin_q brings up a valid point above, let's not change the variable names since those are injected automatically and theme system wide. That issue's patch should change all the template files at the same time, so we don't break HEAD. That also means we cannot change them here without breaking HEAD as well.

{#
/**
 * @file
 * Default theme implementation to display a block.
 * 
 * To add custom classes, separate and print the "attributes.class" variable
 * prior to printing the entire "attributes" variable. Make note of the spacing.
 * @code
 *   <div class="my-custom-class {{ attributes.class }}"{{ attributes }}>
 * @endcode
 *
 * ... rest of docblock
 */
#}
<div{{ attributes }}>
  {{ title_prefix }}
  {% if label %}
    <h2{{ title_attributes }}>{{ label }}</h2>
  {% endif %}
  {{ title_suffix }}
  {{ content }}
</div>
jenlampton’s picture

yes yes and yes.

But let's also keep in mind that we want to have the title (and other template variables) be drillable *everywhere* as well, so in a perfect world {{ title }} prints the whole thing, but you can still get title.prefix, title.suffix, title.label and title.attributes as well. This only ties in tangentially to attributes (title.attributes) but will also need to be changed globally.

{#
/**
 * @file
 * Default theme implementation to display a block.
 * 
 * To add custom classes, separate and print the "attributes.class" variable
 * prior to printing the entire "attributes" variable. Make note of the spacing.
 * @code
 *   <div class="my-custom-class {{ attributes.class }}"{{ attributes }}>
 * @endcode
 *
 * ... rest of docblock
 */
#}
<div{{ attributes }}>
  {{ title.prefix }}
  {% if label %}
    <h2{{ title.attributes }}>{{ title.label }}</h2>
  {% endif %}
  {{ title.suffix }}
  {{ content }}
</div>
markhalliwell’s picture

@jenlampton, yes I'd like to have the title stuff be drillable too. But given that is done globally and not part of this issue, we can't change them here just yet. That will need to be a separate issue.

no2e’s picture

We should use section instead of div, at least when a title/heading is used. Otherwise we get a wrong outline when we have several blocks but not all of them with heading.

Example: two blocks, the first with heading, the second without:

<!-- block 1 -->
<div>
  <h2>New users</h2>  
  <p>…</p>
</div>
<!-- block 2 -->
<div>
  <p>…</p>  
</div>

Here the second block is in scope of the heading "New users" (from the first block), which is, of course, wrong (the content of the second block has nothing to do with new users).

By using a sectioning content element, we can exactly define the scope of the heading:

<!-- block 1 -->
<section>
  <h2>New users</h2>  
  <p>…</p>
</section>
<!-- block 2 -->
<div>
  <p>…</p>  
</div>

Now this heading only applies to its block. Good.

But now the second block longs for a heading, and most of the time it will find one, but not necessarily a correct one (depending on the specific page structure). So it would be even better to use section there, too. But this issue is not as important as the one described. Whenever a heading (h1-h6) is used, use section (or a more appropriate sectioning content element: article/aside/nav), otherwise you might mess up the outline.

sun’s picture

Issue summary: View changes

This issue suffers from the scope-creep syndrome:

  1. class="my-custom-class {{ attributes.class }}"{{ attributes }} vs. {{ attributes }}
  2. section vs. div
  3. h2 vs. h3
  4. title_prefix/title_suffix vs. title.prefix/title.suffix
  5. label vs. title.label
  6. {{ content }} vs. <div>{{ content }}</div>

Therefore, I re-opened #1972122: Remove the DIV tag around block content and manually extracted the relevant changes from this patch.

mortendk’s picture

good move sun. :)

MathieuSpil’s picture

Some updating for sanity's sake:

  1. class="my-custom-class {{ attributes.class }}"{{ attributes }} vs.{{ attributes }}
    =>This is a already changed by other ticket and now follows <div{{ attributes.addClass(classes) }}> (so this shouldn't be discussed here)
  2. section vs.div
    => By the logic that blocks can be placed into regions, and blocks can swap places with each other, one can say that this is a specific part of your html-document, that may specifically stand out. And by this definition: "The section element represents a generic section of a document or application. A section, in this context, is a thematic grouping of content. The theme of each section should be identified, typically by including a heading (h1-h6 element) as a child of the section element." I am a fan of using the section-tag
  3. h2 vs. h3
    =>Indeed open for discussion, if we look at how other headers will be implemented, we can say the following:
    • H1: page-titles
    • H2: Title above the comment-form, node-titles when not displayed as a page,
    • H3: comment-title, search-results title,

    I would say that the really obvious argument for titles at menu-blocks can be justified by using a H2 in the block--system-menu-block.html.twig and if not use a H3. On the other hand, it is entirely possible to create blocks with teasers, and if those teasers contain titles, those should be H4's

  4. title_prefix/title_suffix vs. title.prefix/title.suffix
    => title.prefix/title.suffix is not used anywhere in current version, so this should not be discussed here?
  5. label vs. title.label
    => title.label is not used anywhere in current version, so this should not be discussed here?
  6. {{ content }} vs. <div>{{ content }}</div>
    =>The re-opened issue by Sun #1972122: Remove the DIV tag around block content is fixed, so this should no longer be discussed here.

Recreated last patch without changes fixed in #1972122: Remove the DIV tag around block content
Did not yet changed div to section because of no experience in altering the tests.
Also noticing that summary should get update.

star-szr’s picture

Status: Needs work » Needs review

Let's see what tests fail at least. Thanks for the assessment and patch @MathieuSpil!

Status: Needs review » Needs work

The last submitted patch, 49: dream-markup-block-1982244-49.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Liam Morland made their first commit to this issue’s fork.

Liam Morland’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -

I have made an issue fork and merge request that updates block.html.twig:

  • Allow the wrapper element to be customized
  • Default to wrapping blocks with labels in section
  • Allow customizing the label element

The patch could also leave out defaulting to section but keep the other changes. This would mean that nothing would change on its own, but people could use hook_preprocess_HOOK() to change the elements if they want to.

smustgrave’s picture

Component: markup » block.module
Issue tags: +Needs subsystem maintainer review, +Needs tests
smustgrave’s picture

Status: Needs review » Postponed

postponing on the decision over at #3427955: Do not use `article` elements without a header since they seem to be related.