Here's a first shot at a render example, demonstrating D7 render arrays and altering.

Reviews, suggestions appreciated.

CommentFileSizeAuthor
#25 examples.render_example_867256_25.patch26.49 KBrfay
#14 examples.render_example_867256_13.patch20.22 KBrfay
#9 examples.render_example_867256_08.patch12.91 KBrfay
#4 examples.render_example_rfay_867256_03.patch7.14 KBrfay
examples.render_example_rfay.patch7.73 KBrfay

Comments

moshe weitzman’s picture

moshe weitzman commented

Looks good. My one hesitation is render_example_form_alter(). I think there is already confusion between render arrays and form arrays and its best to leave forms out of the learning discussion. Not a big deal tho. Thanks for the example.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Component: Other » Render Example
rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Removed the form_alter portion. The intent *is* to explain about render arrays.

@moshe: Are there links that should be added to this? A good blog post somewhere? I didn't see any links on http://sf2010.drupal.org/conference/sessions/page-render-drill-down-drup...

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
StatusFileSize
new examples.render_example_rfay_867256_03.patch7.14 KB

Oops. Seem not to have attached the patch.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Hmm...

I notice one big thing I didn't know about altering Render Arrays before:

There is no container to which I can properly append the item I'm rendering in.

If altering a block, the block->content contains *just* the stuff that was in the block (there is no container in there). So it might contain *just* a list of menu items, or *just* form elements.

Bottom line: This example ends up adding a form as an element in a

    on a menu, or a form as an element in a form on a form block. None of those are what's intended. Also, I'm adding the page fieldset to the the footer, which is a little awkward, but not as bad as the above.

    @moshe: Any suggestions about how to do this better?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I think I'm going to have to put this down for now. Here's what I found:

1. No matter what I add to a block, it gets processed the way the block wanted to be processed. So If I add something into a menu block, it comes out inside the

    . If I add to a block which is a form, it comes out inside the form. Obviously both bad for the HTML.

    2. @Moshe suggested pushing the existing block down a level and then making it a peer of what I was adding:

    • region
      • block1
      • pseudoblock
        • original block2
        • my_addition

    In that case I get errors because the default Bartik theme assumes that everything below the region level is an official block that has a #block in it (template.php). This could just be an oversight/assumption in Bartik that should be fixed.

    So the right way forward on this might be to fix the Bartik issue.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

The approach in #6 failed probably just because of the assumptions Bartik was making. The Bartik code in its theme_page_alter() should probably be moved to block module, or at least it could be more suave and careful. #6 should be re-tried with another theme, and an issue should be filed with Bartik.

But there are other ways of approaching this, as pointed out by @moshe in IRC:

1. Append to the rendered element using #after_build
2. Add another theme_wrappers before the block's own wrapper

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Here's a new version. Not done yet, and tests not done yet, but this demonstrates things like #pre_render, #post_render, #theme_wrappers, etc.

Works fine with themes other than bartik, but emits errors with bartik due to #889100: remove bartik_page_alter() and extra block classes.
Also, filed

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
StatusFileSize
new examples.render_example_867256_08.patch12.91 KB

Appears that I forgot the patch.

Status: Needs review » Needs work

The last submitted patch, examples.render_example_867256_08.patch, failed testing.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Needs review

#9: examples.render_example_867256_08.patch queued for re-testing.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

#889100: remove bartik_page_alter() and extra block classes landed, so this is fine now.

chx’s picture

chx commented
Status: Needs review » Needs work

+ foreach($xpath_array as $xpath) { <= space after foreach

kpr needs a comment explaining it's from devel and what it does.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Needs review
StatusFileSize
new examples.render_example_867256_13.patch20.22 KB

Here's the current version. Includes chx comment from #13, plus lots and lots more. The only think I consider missing is an example with #access and tests for all the new stuff that I added (the actual arrays that are rendered on the new page that's added)

ilo’s picture

ilo commented

I'm reviewing it.. just two notes:
.info file does not declare 'package = Example modules'

and, any particular reason why devel is a dependency of the reder example? (still did no look deep enough to discover myself)

Edited: I mean, in render example, kpr() can be replaced with a copy of dprint_r() function and remove the dependancy.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I'm using kpr() in the code, provided by devel. It's just the way I'm using to present a complex array in a navigable fashion. Open to better ways.

chx’s picture

chx commented
Status: Needs review » Needs work

I think the Krumo dependency from devel is fine. I have no problems with that. As you need to reroll with the info anyways, maybe add "we could use debug() but that's not really nice for such complex arrays". Mentioning debug() is always good :)

ilo’s picture

ilo commented

No problem including devel, probably it is already installed in the system.

One concern about the patch..

this is an abstract from render_example.test

...
    $edit = array(
      'render_example_show_block' => TRUE,
    );
    $this->drupalPost('examples/render_example', $edit, t('Save configuration'));

    $xpath_array = array(
      "//div[@id='sidebar-first']//fieldset[starts-with(@id, 'edit-render-example-block-fieldset')]",
      '//*[@id="content"]//fieldset[contains(@id,"edit-render-example-block-fieldset")]',
    );
    $this->assertRenderFieldset($xpath_array);

But "examples/render_example" has no form at all, it is just an static page, and the form that has to be tested is at: "examples/render_example/altering"..

In fact, locally I'm unable to pass the tests, so.. how does the bot test this module and return no errors?

Edited: These are the results when I run tests locally:
12 passes, 10 fails, 0 exceptions, and 4 debug messages

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

@ilo, there are two separate pages. You probably haven't rebuilt your menu router.

ilo’s picture

ilo commented

@rfay, sure? I'm looking the code:

/**
 * Implementation of hook_menu().
 */
function render_example_menu() {
  $items['examples/render_example'] = array(
    'title' => 'Render Example',
    'page callback' => 'render_example_info',
    'access callback' => TRUE,
  );
...

the path 'examples/render_example' uses the 'render_example_info' callback:

/**
 * Simple basic information about the module; an entry point.
 */
function render_example_info() {
  return t('The render example provides a <a href="!arrays">demonstration of of render array usage</a> and <a href="!alter">using hook_page_alter()</a> to make various changes on a page.', array('!arrays' => url('examples/render_example/arrays'), '!alter' => url('examples/render_example/altering')));
}

I see no form in this function, it is just a text.. but the testcase is submitting a form in that url:

  $this->drupalPost('examples/render_example', $edit, t('Save configuration'));

Is me who have not rebuild the menu? can you verify that locally tests pass?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Sorry, I was misreading your statement.

I have been working on the code, not the tests. I haven't returned to the tests since I expanded to 3 pages.

And unfortunately PIFR does *not run* new tests associated with a patch. Pretty weird. Makes you feel pretty vulnerable. If you look at http://qa.drupal.org/pifr/test/90164, you'll see no running of render example tests.#879662: Use test.directory.review instead of module list

ilo’s picture

ilo commented

Ok! I was affraid my Drupal setup was doing something nasty. Sorry, my fault then, I should had look for something in the pift queue. Definitely I need to do a quick update on too many things!

Will you work on it or do I try to fix the testscase?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I won't be able to work on this until at least Friday, so I'd love you to have a go at it!

Note that there are no tests at all for the new stuff (the example renders).

ilo’s picture

ilo commented

Yeah, the problem is, I have no idea about the rendering stuff yet, so I'd have to sit down and learn about it :)

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Needs review
StatusFileSize
new examples.render_example_867256_25.patch26.49 KB

OK, here's the version I'll commit if it's green (which it will be because the tests don't get run :-)

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Fixed

Committed to HEAD: http://drupal.org/cvs?commit=427658

All reviews and discussion are welcome. I'm writing an article about this and I'll get a d.o handbook page for rendering going as well.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Fixed » Needs work

I forgot that introducing devel as a dependency would break this. So it failed the Bot. I'm going to have to find out from somebody if it's even possible to test with a contrib module as a dependency of another contrib module.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Fixed

Fixed it with a 'dependencies' => array('devel') in the getInfo() method. Thanks to berdir!

Head is now fixed.

Status: Fixed » Closed (fixed)

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