Port render_example to D8

I'll start working here. Firstly updating #1 to the current version of drupal 8, then I'll continue improving this example.

Hello!
After a little bit of work I updated the previous patch to the current versión of D8, basically that are the changes:

• Moved the .info file to info.yml
• Moved hook_menu to the new routing system.
• Moved moved the variables to the new config system.
• Moved the current menu callbacks to Controllers and Forms

Tests still failing :( . Because we haven't available hook_page_alter anymore (https://www.drupal.org/node/2357755). I think we should move that example (Altering example) placing the render arrays into the header using (https://api.drupal.org/api/drupal/core!modules!system!theme.api.php/func...) or same with the button.

What do you think?

Thanks!

Thanks, jlbellido and marvil07. I've resumed work from the progress in #4. So far I've gone through the description page (to use a render array for lists) and the arrays page. I plan on tackling the hooks through the end of this week. I've been trying to leave a lot of comments.

I found the original example odd since it didn't use the render array structure in the code, but rather, used a custom object that was then processed into a render array. I've updated this part.

There was also a bug that has since been resolved where the work-around was causing issues with duplicate content. This should be fixed.

I don't want to commit the code right now because it's in progress, but I wanted to poke my head in and say this thread is not dead!

I have gone through all of the examples, but I haven't touched the testing file. I'll try to take a look at that.

I don't really like a lot of the examples of altering arrays and agree with the change to get rid of hook_page_alter(). That being said, to accomplish these examples, I think we need to use hook_preprocess_page() and hook_preprocess_block which does almost the same thing. I don't really like that use, but it works.

I tried to re-organize the functions in the file to be more in line with the example pages.

Please have a look and let me know. Might be worth trying to get something in as v1.0 since these patches are unwieldy.

Please, could you attach teh interdiff? Just for simplify the review.

Thanks!

I'm going to take stab at updating this example. One question I have, before I get to deep into it. Right now the example requires the Devel module be installed. This is so that it it can use the associated kpr() and kint() functions in order to display the content of various renderable arrays. Both of which print arrays in a collapsible, formatted, structure. This does make it a bit easier to read. But it then requires that Devel is installed. Are we okay with this dependency for people wanting to use the Render API example? Or should we come up with a different way of showing the information.

Okay, I've been working on some Render API related tutorials for Drupalize.Me and needed some example code to go along with them. Here's my version of this so far. I haven't touched the tests yet, but everything else should be in working order. I started from the patch in #7, and then tried to add some more inline comments, and ensure things are up-to-date for both current Drupal 8 standards and best practices, as well as standards for this project.

In the process I removed a couple of examples that were basically duplication. I didn't think it was necessary to demonstrate the same thing more than once. I also added an example of adding a new RenderElement plugin, '#type' => 'marquee', as this is likely to be a relatively common thing for module developers to want to do. I also made the Devel module an optional dependency instead of a hard dependency.

I ran this through phpcs, and there's one violation but I believe that it's a false positive.

FILE: ...amples/render_example/src/Controller/RenderExampleController.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 467 | ERROR | If the line declaring an array spans longer than 80
     |       | characters, each element should be broken into its own
     |       | line (Drupal.Array.Array.LongLineDeclaration)
----------------------------------------------------------------------

I anticipate the tests will fail. But wanted to share where I'm at incase someone else has time to work on the tests. Not sure I'll get around to it for at least a few weeks.

Thanks much, @eojthebrave

OK, so I re-rolled this and found a few things.

I'm pretty sure this test run will still fail...

1. The render example links do not appear in the tool menu block, so it needs a render_example.links.menu.yml file specifying that. We also need to add a test to make sure this link does appear when the module is installed.
2. The alter hooks page doesn't save the order or selections for the favorite color tables. Needs a test.

3. +++ b/render_example/src/Tests/RenderExampleTest.php
   @@ -0,0 +1,136 @@
   +class RenderExampleTest extends WebTestBase {
   

   Needs to be a BrowserTestBase test.

4. +++ b/render_example/src/Tests/RenderExampleTest.php
   @@ -0,0 +1,136 @@
   +  public static $modules = array('devel', 'render_example', 'node');
   

   We should have a test without enabling devel that verifies that everything works.

5. +++ b/render_example/src/Tests/RenderExampleTest.php
   @@ -0,0 +1,136 @@
   +  protected function assertRenderResults($xpath_array) {
   ...
   +  protected function assertRenderedText($xpath_array) {
   

   I'm pretty sure these will be replaced by the web assert object from BTB::assertSession().

6. +++ b/render_example/templates/description.html.twig
   @@ -0,0 +1,21 @@
   +<p>Add some text here that gives an overview of the render API system? ......................................</p>
   

   Documents itself. :-) Needs more explanation.

I'm working on this.

This patch deals with #14.1, .3, .4, .5.

The tests pass, but we still need tests for the arrays/ path and the drag and selection table parts.

The table parts should really be in fapi_example, to demo how to save the state and so forth. I can't see where they demo how to alter the form here.

Ewps. Forgot the patch.

The table parts should really be in fapi_example, to demo how to save the state and so forth. I can't see where they demo how to alter the form here.

Yeah, that makes sense to me.

Thanks for working on the tests for this.

We don't get too far into page or render caching and we need to talk about it more, so I've made a follow-up: #2925424: Expand examples for more page/render caching use-cases I'm not sure if this info should be here or in page_example. Maybe page caching should be in page_example and render array chunk caching should be here. Decide that in the other issue. :-)

+++ b/render_example/tests/src/Functional/RenderExampleTest.php
@@ -0,0 +1,126 @@
+  /**
+   * {@inheritdoc}
+   *
+   * @TODO: I'm not sure at all we need 'node'. Currently we need it because we
+   *   create a new user with the 'Access content' permission.
+   */
+  public static $modules = ['render_example'];

Is this @TODO still relevant? It doesn't look like 'node' is in the modules list.

I believe we can remove the #tabledrag stuff as that is all covered by the tabledrag_example module now.

Should we also remove the #tableselect stuff from here before committing this? And at least open up a new issue to add that into the formapi example? And then continue that work there?

Other than that the changes here look good to me. Thanks for helping to get the tests sorted out.

CS, properly set up config, added @todos about #prerender, etc.

Still has the tables, which we'll move eventually.

And no, that @todo is no longer relevant since the module info file says it's got a dependency on node.

Grr. I managed to undo the last few commits so they're not in the patch.

Let's try that again.

1. +++ b/render_example/src/Controller/RenderExampleController.php
   @@ -64,6 +64,21 @@ class RenderExampleController extends ControllerBase {
   +    // CSS and JavaScript libraries can be attached to elements in a renderable
   +    // array. This way, if the element ends up being rendered and displayed you
   +    // know for sure the CSS/JavaScript will also be included. But, if the for
   +    // some reason the element isn't ever rendered then Drupal can skip the
   +    // unnecessary extra files.
   

   "But, if the for some reason ..." you can remove the "the" in this sentence.

2. +++ b/render_example/src/Controller/RenderExampleController.php
   @@ -277,7 +292,8 @@ class RenderExampleController extends ControllerBase {
   +    // @todo: This doesn't work, we need to fix it.
   +    // $build['#pre_render'] = [static::class . '::preRender'];
   

   I believe the syntax for this is, $build['#pre_render'] = [static::class, 'preRender']; Though I haven't actually tested that yet.

@eojthebrave hope this is what you are looking for... The patch has the corrections mentioned in #27

I missed a couple of things in the previous review that I've addressed in this patch. One, is that our implementation of '#theme' => 'item_list' didn't work. The other, is adding back some code that was previously a part of this module which allows the demonstration page to display both the rendered, and unrendered array next to each other.

At this point I would be totally comfortable with this being committed and new issues being opened for any additional bugs that come up.

Oh, and here's an interdiff for #29.

Re-roll.

Fixed route paths.

Committed. Thanks!