API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

Currently says:

variables: Only used for #theme in render array: an array of variables, where the array keys are the names of the variables, and the array values are the default values if they are not given in the render array. Template implementations receive each array key as a variable in the template file (so they must be legal PHP/Twig variable names). Function implementations are passed the variables in a single $variables function argument.

Could really do to explain that the variable name you define here is the name without the #, so that you have this system:

Defined in hook_theme():

    'book_navigation' => array(
      'variables' => array('book_link' => NULL),
    ),

Use in a render array:

      $build['book_navigation'] = array(
        '#theme' => 'book_navigation',
        '#book_link' => $node->book,
CommentFileSizeAuthor
#9 add-more-detail-to-hook_theme-variables-2712405-9.patch909 bytesleolandotan
#9 interdiff-2712405-6-9.txt1.32 KBleolandotan
#6 interdiff-2712405-3-6.txt924 bytesleolandotan
#6 add-more-detail-to-hook_theme-variables-2712405-6.patch1.21 KBleolandotan
#3 add-more-detail-to-hook_theme-variables-2712405-3.patch1.2 KBleolandotan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: Unassigned » leolandotan
leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: leolandotan » Unassigned
Status: Active » Needs review
FileSize
add-more-detail-to-hook_theme-variables-2712405-3.patch1.2 KB

Here I tried to add the recommendation of @joachim including the sample implementation code.

Hope everything is in order.

Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks for the patch! But I think this is way more explanation than we need. Can we cut it down to just one sentence? Also please wrap it into the paragraph that is already there.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: Unassigned » leolandotan
leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: leolandotan » Unassigned
Status: Needs work » Needs review
FileSize
add-more-detail-to-hook_theme-variables-2712405-6.patch1.21 KB
interdiff-2712405-3-6.txt924 bytes

Thanks for the review! Here I have cut down the explanation following from what's on the issue description and just included the new explanation into the paragraph before it.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks, but...

+++ b/core/lib/Drupal/Core/Render/theme.api.php
@@ -1110,7 +1110,20 @@ function hook_page_bottom(array &$page_bottom) {
+ *     argument. The variable name you define here is the name without the #, so
+ *     you have this system:
+ *     Defined in hook_theme():
+ *     @code
+ *     'book_navigation' => array(
+ *       'variables' => array('book_link' => NULL),
+ *     ),
+ *     @endcode
+ *     Use in a render array:
+ *     @code
+ *     $build['book_navigation'] = array(
+ *       '#theme' => 'book_navigation',
+ *       '#book_link' => $node->book,
+ *     @endcode

Again, this is WAY more than I think we need.

How about just saying something like:

If you are using these variables in a render array, prefix the variable names defined here with a #.

I really don't think we need all these code samples...

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: Unassigned » leolandotan
leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: leolandotan » Unassigned
Status: Needs work » Needs review
FileSize
interdiff-2712405-6-9.txt1.32 KB
add-more-detail-to-hook_theme-variables-2712405-9.patch909 bytes

Thanks for your feedback @jhodgdon! My bad on not getting the "this is WAY more than I think we need" part right away.

Here I have removed the code samples and followed your recommendation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community

That looks good to me. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed ba83d07 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed 44f8f8f on 8.2.x 
    Issue #2712405 by leolando.tan, jhodgdon: more detail needed for...

  • alexpott committed ba83d07 on 8.1.x 
    Issue #2712405 by leolando.tan, jhodgdon: more detail needed for...

Status: Fixed » Closed (fixed)

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