Derived from #1183042: Regression: Add WAI-ARIA roles to Core blocks, allow module maintainers to inject attributes and classes into blocks by adding "attributes" to hook_block_view(). system_preprocess_block() is a good example of why this is a good thing as it saves us having to write another preprocess function.

Files: 
CommentFileSizeAuthor
#9 hook_block_view-attributes.patch7.24 KBmichaelfavia
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_block_view-attributes_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
hook_block_view-attributes.patch7.16 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 33,296 pass(es).
[ View ]

Comments

Right now, if you have a module where you're creating a block where you need a specific block class, you need to use:

<?php
mymodule_block_view
() {
 
$block['subject'] = t('Hi thare!');
 
$block['content'] = '<div class="mymodules-specific-class"'>' . $content . '</div>;
  return
$block;
}
?>

It makes infinitely more sense to use:
<?php
mymodule_block_view
() {
 
$block['subject'] = t('Hi thare!');
 
$block['content'] = $content;
 
$block['attributes']['class'][] = 'mymodules-special-class';
  return
$block;
}
?>

Thanks Rob!

So, my question is can this be done for nodes too? And anything else?

Subscribe :)

I was thinking about this last night as @Jacine does make a good point. The key difference between Blocks and Nodes, however, is that Blocks are not Entities. Blocks are containers for content on your site. They hold content, doesn't matter what it is, they just display it. This means we either:

Add a "attributes" parameter to hook_block_view()
This doesn't really fit in with the rest of the Drupalism we have, which Jacine pointed out.
Turn Blocks into Entities
But Blocks are just containers for other content, so they don't really make sense as entities.
Turn Blocks into Renderable Arrays
And this is the solution I came to. Having Blocks as renderable arrays mean that we can have any kind of content in there, we can stick in any attributes we want, and it fits with the rest of our Drupal ideology.

...... or am I just crazy?

Not crazy at all. I thought blocks were going to turn into entities though? Anyway, blocks should definitely become render arrays. I think the only reason they weren't switched over in D7 was because there wasn't time. It's totally weird that they are not right now.

The main thing here that I am concerned about is that ALL attributes are done the same way for consistency. I still worry about being able to manipulate them fully in preprocess functions. I have run into some funky situations in D7 where pre render functions destroy the theme's ability to make preprocess/preprocess changes. Though, if anyone screws around with attributes in that way, I guess that would be pretty dumb.

If we made blocks into renderable elements (which i am in support of), we could do:

#type = block
#attributes][class][] = my-block-custom-class
#title = My block title // This is currently 'subject'
#title_display = invisible // If you want to hide the title
... // the rest of the content of the block.

Cross post with #430886: Make all blocks fieldable entities

Blocks are going toward entities. The block rendering should be a render array. The other issue is now focused on making the custom block entities. If we end up with all blocks as entities then blocks will return render arrays.

@Rob Loach that statement isn't true. $block['content'] expects a render array. see http://api.drupal.org/api/drupal/modules--block--block.module/function/_...

Status:Needs review» Needs work
Issue tags:+Novice

Thanks for your work on this patch. Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

Tagging as novice for the task of rerolling the Drupal 8.x patch.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

Status:Needs work» Needs review
StatusFileSize
new7.24 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_block_view-attributes_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled post core move.

I'd like to point out that block contents are already renderable arrays. If you have a module that is producing a block and its contents aren't a render array, that's arguably a bug.

That still doesn't solve the problem of wanting to add attributes to the block.tpl wrapper.

Still, I think converting blocks to entities sounds more interesting. And would likely mean any work here would need to be undone or re-done.

I'd like to point out that block contents are already renderable arrays. If you have a module that is producing a block and its contents aren't a render array, that's arguably a bug.

@JohnAlbin So why aren't we using render() for $content in block.tpl.php? Did this just happen in D8 or is it this way in D7 too?

Should we just postpone this for now?

Status:Needs review» Needs work

It is correct that the preferred return from hook_block_view is a render array in D7. We didn't convert all blocks because it didn't seem that important for the simpler ones, and it takes time to push those patches in.

I think block.tpl.php does not use render() becasause it is a called as a theme_wrapper, not by theme(). So it gets handed already rendered $content and not a render array. All it does is "wrap" that content in the usual block markup.

So this issue seems 'works as designed' or needs to focus more on converting legacy blocks.

So I haven't tested this - if a block returns a render array, can it add affect attributes in block.tpl.php or not currently?

Title:Add "attributes" to hook_block_view()Ensure render arrays properly handle #attributes and add them there instead of preprocess hooks
Priority:Normal» Major
Issue tags:-Novice

I'm going to commit #1183042: Regression: Add WAI-ARIA roles to Core blocks. Using preprocess is not ideal at all, but hook_preprocess_node() shows that's the current standard. If everything works already, then we can use this issue to track converting older code that's relying on preprocess to use it.

Re-titling and bumping to major.

if a block returns a render array, can it add affect attributes in block.tpl.php or not currently?
Not really.
A quick fix might be to change $build in _block_get_renderable_array() to basically stuff everything except the theme wrapper in a child element, and then have the function provide a build alter to add our #attributes or #attributes_array to the parent. (#attributes would need to be preprocessed.)

Oops, _block_get_renderable_array() is D7.
In D8 there already is a child element for content and thus a parent element to have its very own attributes. So why not use #theme now instead of #theme_wrappers and convert the block preprocessors?

Currently you would still have to rely on page altering to add attributes to a main block element.

I guess it's alright to pass #attributes next to #children to a theme wrapper, so no urgent need to switch to #theme. (See template_preprocess_region() for the #children and #region variant.)

So if it were alright to have hook_block_view() return a renderable block with a 'content' child, we could add both "content attributes" and "wrapper attributes" there. Then block_get_renderable_region() would add the final theme wrapper and template_preprocess_block() would populate $attributes_array.

Such a patch would touch every implementation of hook_block_view() and hook_block_view_alter(), but would allow for early initialization of WAI-ARIA role attributes.

I created #1402250: Allow modules to classify blocks when defining them, so WAI-ARIA roles can automatically be added in the theme layer to follow up on the WAI-ARIA roles.

It may or may not wind up relying on this issue (I personally prefer the implementation that doesn't), but either way I think this issue is a good one to solve :) Having to use a preprocess function to add an attribute is definitely annoying.

Status:Needs work» Needs review
Issue tags:-block.tpl.php, -hook_block_view

#9: hook_block_view-attributes.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+block.tpl.php, +hook_block_view

The last submitted patch, hook_block_view-attributes.patch, failed testing.

Status:Needs work» Postponed
Issue tags:+Blocks-Layouts

This is almost exclusively about blocks, and #1535868: Convert all blocks into plugins will drastically change all of this, possibly even rendering it obsolete, so postponing.

Status:Postponed» Active

Unpostponing. I also re-opened #1183042: Regression: Add WAI-ARIA roles to Core blocks to specifically track the regression of the role attribute, though maybe this issue will fix that.

Priority:Major» Normal

This should be more than possible now.
BlockPluginInterface::build() (effectively the replacement for hook_block_view) now requires returning a render array.