In the docblock for hook_block_view:

<?php
/**
*  content: The content of the block's body. This may be a renderable array (preferable) or a string containing *  rendered HTML content.
*/
?>

But in the example code, both block examples return strings. Since a renderable array is preferred, at least one of the block examples should return a renderable array for its content.
Files: 
CommentFileSizeAuthor
#15 hook_fix-1034248-15.patch1.76 KBjn2
PASSED: [[SimpleTest]]: [MySQL] 29,417 pass(es).
[ View ]
#7 docs_1034248.patch1.97 KBdrewish
PASSED: [[SimpleTest]]: [MySQL] 31,563 pass(es).
[ View ]
#4 1034248.patch1.61 KBjhodgdon
PASSED: [[SimpleTest]]: [MySQL] 31,554 pass(es).
[ View ]

Comments

Title:Documentation problem with hook_block_viewhook_block_view example should return a render array

Seems like a reasonable suggestion. Thanks!

Though if the example from hook_block_view is pulled from node_block_view, shouldn't node_block_view also be switched to use the render API?

Should *have* been. If it isn't by now, it's probably too late. But if we do change the example here to something other than what node_block_view() does, we should verify that the example works.

Status:Active» Needs review
StatusFileSize
new1.61 KB
PASSED: [[SimpleTest]]: [MySQL] 31,554 pass(es).
[ View ]

I took an example from the menu module, which seemed to be the best one out there.

That's definitely a valid example but I'm not sure it's the most helpful since it delegates to a helper function and doesn't show the format of the render array.

Status:Needs review» Needs work

True. OK, we need a better example.

Status:Needs work» Needs review
StatusFileSize
new1.97 KB
PASSED: [[SimpleTest]]: [MySQL] 31,563 pass(es).
[ View ]

Does blog_block_view() seem like a better example? The downside of it is that it ignores $delta. Do we try to always use a core example? Or can we roll custom examples?

Custom examples that are concise and clear are great. Maybe the Examples project would be a good place to look? http://drupal.org/project/examples

Status:Needs review» Needs work

And what I would do is take the function body from
http://api.drupal.org/api/examples/block_example--block_example.module/f...
and put it directly into the hook_block_view body as well rather than calling a second function, for better illustration. And probably the comments don't need to be quite so verbose.

That is a bit of an odd example.

If I am reading the patch correctly, the $block array is only returned sometimes; at other times, when the if statement is not TRUE, there is no return at all. I think this needs to be corrected before going in the docs to always return an array, even if empty.

RE #11 - which example are you referring to by "this"?

As I understand, the above patch when applied results in the final example:

function hook_block_view($delta = '') {
  // This example comes from blog.module which only provides a single block
  // so it ignores the $delta parameter.
  global $user;
  if (user_access('access content')) {
    $result = db_select('node', 'n')
      ->fields('n', array('nid', 'title', 'created'))
      ->condition('type', 'blog')
      ->condition('status', 1)
      ->orderBy('created', 'DESC')
      ->range(0, variable_get('blog_block_count', 10))
      ->addTag('node_access')
      ->execute();
    if ($node_title_list = node_title_list($result)) {
      $block['subject'] = t('Recent blog posts');
      $block['content']['blog_list'] = $node_title_list;
      $block['content']['blog_more'] = array(
        '#theme' => 'more_link',
        '#url' => 'blog',
        '#title' => t('Read the latest blog entries.'),
      );
      return $block;
    }
  }
}

This is the example function that has an inconsistent return.

Ah. We're asking for a different patch to be made with a better example. That's why this patch has been set to "needs work". See #9-10 for suggested example to include in the new patch.

Version:7.x-dev» 8.x-dev
Status:Needs work» Needs review
StatusFileSize
new1.76 KB
PASSED: [[SimpleTest]]: [MySQL] 29,417 pass(es).
[ View ]

How about adapting the current example to return renderable arrays? It's a good example with no helper functions, concise and clear, doesn't ignore $delta, and has two places for renderable arrays.

Here's a patch that does that.

That is a good example and appears to be quite clear. My only small nit is how odd 'node.module.' looks upon first reading of the comments.

node.module - agreed it looks a bit odd with a . after it, but nonetheless that is standard in Drupal comments.

That example looks OK to me. Has someone tested it to make sure it actually works?

Yes, I checked it out. Made the changes on a local Git branch of the latest version of core and found no problems in the site. I also ran the SimpleTest tests for the node module, all green.

Status:Needs review» Reviewed & tested by the community

Sounds good then, thanks!

Status:Reviewed & tested by the community» Needs work

+++ b/modules/block/block.api.php
@@ -205,23 +205,27 @@ function hook_block_save($delta = '', $edit = array()) {
-      $block['content'] = theme('feed_icon', array('url' => url('rss.xml'), 'title' => t('Syndicate')));
+      $block['content'] = array(
+        '#theme' => 'feed_icon',
+        '#url' => 'rss.xml',
+        '#title' => t('Syndicate'),
+      );

Is that actually correct? We lost a call to url() in the process it seems.

Now, I did some grepping (read: investigation) and found this:

<?php
modules
/block/block.api.php:      $block['content'] = theme('feed_icon', array('url' => url('rss.xml'), 'title' => t('Syndicate')));
modules/node/node.module:      $block['content'] = theme('feed_icon', array('url' => 'rss.xml', 'title' => t('Syndicate')));
?>

Note that one call uses url('rss.xml') and one uses just 'rss.xml'. In other words, it looks like we might have fixed a bug along the way -- whether that was intentional or not. I have not tested the update example.

Last but not least, shouldn't we make sure core follows our examples? :)

Thoughts before I commit this patch?

Status:Needs work» Needs review

I checked into it some, and there does indeed seem to be a bug in modules/block/block.api.php. I added the call to url() to node.module, and the feed icon doesn't work right with that inserted. (I ran the node tests, and those all pass with any of these versions.)

In Firefox 4, with the call to url() inserted, the browser looks for rss.xml and returns a 'Server not found' page. With node.module unchanged, or with it changed as in the patch, it looks for example.com/rss.xml and shows the Syndicate page.

Yes, I agree that core should follow the examples. :) If this is confirmed, I'll make a patch for node.module reflecting this change.

Status:Needs review» Reviewed & tested by the community

Looking at theme_feed_icon() http://api.drupal.org/api/drupal/includes--theme.inc/function/theme_feed... ... It looks like the url theme variable is passed into l(), so it should either be an internal path or an external URL. So there is no need for wrapping with url().

So, the current node_block_view() is correct http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...

And the example hook function body in the patch in #15 is also correct, and makes the example here match the code in node.module, so I will set this patch back to RTBC.

Status:Reviewed & tested by the community» Fixed

Thanks for investigating this a bit more, jn2 and jhodgdon. That was helpful.

I've committed the documentation fix to 7.x and 8.x. Thanks!

Status:Fixed» Closed (fixed)
Issue tags:-Novice

Automatically closed -- issue fixed for 2 weeks with no activity.