Issue #2443361 by joelpittet, Cottser, sqndr, lauriii, Manuel Garcia, Wim Leers: Remove theme_book_link, make book tree align with MenuLinkTree build

core/modules/book/book.module

@@ -63,11 +63,7 @@ function book_theme() {
       'variables' => array('book_link' => NULL),
     ),
     'book_tree' => array(
-      'render element' => 'tree',
+      'variables' => array('items' => array(), 'attributes' => array()),
-    ),
-    'book_link' => array(
-      'render element' => 'element',
-      'function' => 'theme_book_link',
     ),
     'book_export_html' => array(
       'variables' => array('title' => NULL, 'contents' => NULL, 'depth' => NULL),
@@ -499,37 +495,6 @@ function template_preprocess_book_node_export_html(&$variables) {
   $variables['content'] = $variables['node']->rendered;
 }
 
-/**
- * Implements template_preprocess_HOOK() for book-tree.html.twig.
- */
-function template_preprocess_book_tree(&$variables) {
-  $variables['tree'] = $variables['tree']['#children'];
-}
-
-/**
- * Returns HTML for a book link and subtree.
- *
- * @param array $variables
- *   An associative array containing:
- *   - element: Structured array data for a book link.
- *
- * @ingroup themeable
- */
-function theme_book_link(array $variables) {
-  $element = $variables['element'];
-  $sub_menu = '';
-
-  if ($element['#below']) {
-    $sub_menu = drupal_render($element['#below']);
-  }
-  $element['#localized_options']['set_active_class'] = TRUE;
-  /** @var \Drupal\Core\Url $url */
-  $url = $element['#url'];
-  $url->setOptions($element['#localized_options'] + $url->getOptions());
-  $output = \Drupal::l($element['#title'], $url);
-  return '<li' . new Attribute($element['#attributes']) . '>' . $output . $sub_menu . "</li>\n";
-}
-
 /**
  * Determines if a given node type is in the list of types allowed for books.
  *

core/modules/book/src/BookManager.php

@@ -16,6 +16,7 @@ use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\StringTranslation\TranslationInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Template\Attribute;
 use Drupal\node\NodeInterface;
 
 /**
@@ -501,20 +502,46 @@ class BookManager implements BookManagerInterface {
    * {@inheritdoc}
    */
   public function bookTreeOutput(array $tree) {
-    $build = array();
+    $items = $this->buildItems($tree);
-    $items = array();
 
-    // Pull out just the book links we are going to render so that we
+    $build = [];
-    // get an accurate count for the first/last classes.
+
-    foreach ($tree as $data) {
+    if ($items) {
-      if ($data['link']['access']) {
+      // Make sure drupal_render() does not re-order the links.
-        $items[] = $data;
+      $build['#sorted'] = TRUE;
-      }
+      // Get the book id from the last link.
+      $item = end($items);
+      // Add the theme wrapper for outer markup.
+      // Allow menu-specific theme overrides.
+      $build['#theme'] = 'book_tree__book_toc_' . $item['original_link']['bid'];
+      $build['#items'] = $items;
+      // Set cache tag.
+      $build['#cache']['tags'][] = 'config:system.book.' . $item['original_link']['bid'];
     }
 
-    $num_items = count($items);
+    return $build;
-    foreach ($items as $i => $data) {
+  }
+
+  /**
+   * Builds the #items property for a book tree's renderable array.
+   *
+   * Helper function for ::bookTreeOutput().
+   *
+   * @param array $tree
+   *   A data structure representing the tree.
+   *
+   * @return array
+   *   The value to use for the #items property of a renderable menu.
+   */
+  protected function buildItems(array $tree) {
+    $items = [];
+
+    foreach ($tree as $data) {
       $class = ['menu-item'];
+      // Generally we only deal with visible links, but just in case.
+      if (!$data['link']['access']) {
+        continue;
+      }
       // Set a class for the <li>-tag. Since $data['below'] may contain local
       // tasks, only set 'expanded' class if the link also has children within
       // the current book.
@@ -528,30 +555,24 @@ class BookManager implements BookManagerInterface {
       // Set a class if the link is in the active trail.
       if ($data['link']['in_active_trail']) {
         $class[] = 'menu-item--active-trail';
-        $data['link']['localized_options']['attributes']['class'][] = 'menu-item--active-trail';
       }
 
       // Allow book-specific theme overrides.
-      $element['#theme'] = 'book_link__book_toc_' . $data['link']['bid'];
+      $element = [];
-      $element['#attributes']['class'] = $class;
+      $element['attributes'] = new Attribute();
-      $element['#title'] = $data['link']['title'];
+      $element['attributes']['class'] = $class;
+      $element['title'] = $data['link']['title'];
       $node = $this->entityManager->getStorage('node')->load($data['link']['nid']);
-      $element['#url'] = $node->urlInfo();
+      $element['url'] = $node->urlInfo();
-      $element['#localized_options'] = !empty($data['link']['localized_options']) ? $data['link']['localized_options'] : array();
+      $element['localized_options'] = !empty($data['link']['localized_options']) ? $data['link']['localized_options'] : [];
-      $element['#below'] = $data['below'] ? $this->bookTreeOutput($data['below']) : $data['below'];
+      $element['localized_options']['set_active_class'] = TRUE;
-      $element['#original_link'] = $data['link'];
+      $element['below'] = $data['below'] ? $this->buildItems($data['below']) : [];
+      $element['original_link'] = $data['link'];
       // Index using the link's unique nid.
-      $build[$data['link']['nid']] = $element;
+      $items[$data['link']['nid']] = $element;
-    }
-    if ($build) {
-      // Make sure drupal_render() does not re-order the links.
-      $build['#sorted'] = TRUE;
-      // Add the theme wrapper for outer markup.
-      // Allow book-specific theme overrides.
-      $build['#theme_wrappers'][] = 'book_tree__book_toc_' . $data['link']['bid'];
     }
 
-    return $build;
+    return $items;
   }
 
   /**

core/modules/book/src/BookManagerInterface.php

@@ -255,8 +255,7 @@ interface BookManagerInterface {
    * @return array
    *   A structured array to be rendered by drupal_render().
    *
-   * @todo This was copied from menu_tree_output() but with some changes that
+   * @see \Drupal\Core\Menu\MenuLinkTree::build
-   *   may be obsolete. Attempt to resolve the differences.
    */
   public function bookTreeOutput(array $tree);
 

core/modules/book/templates/book-tree.html.twig

@@ -1,16 +1,44 @@
 {#
 /**
  * @file
- * Default theme implementation for a book tree.
+ * Default theme implementation to display a book tree.
  *
  * Returns HTML for a wrapper for a book sub-tree.
  *
  * Available variables:
- *   - tree: An HTML string containing the tree's items.
+ * - items: A nested list of book items. Each book item contains:
- *
+ *   - attributes: HTML attributes for the book item.
- * @see template_preprocess_book_tree()
+ *   - below: The book item child items.
+ *   - title: The book link title.
+ *   - url: The book link URL, instance of \Drupal\Core\Url.
  *
  * @ingroup themeable
  */
 #}
-<ul>{{ tree }}</ul>
+{% import _self as book_tree %}
+
+{#
+  We call a macro which calls itself to render the full tree.
+  @see http://twig.sensiolabs.org/doc/tags/macro.html
+#}
+{{ book_tree.book_links(items, attributes, 0) }}
+
+{% macro book_links(items, attributes, menu_level) %}
+  {% import _self as book_tree %}
+  {% if items %}
+    {% if menu_level == 0 %}
+      <ul{{ attributes }}>
+    {% else %}
+      <ul>
+    {% endif %}
+      {% for item in items %}
+        <li{{ item.attributes }}>
+          {{ link(item.title, item.url) }}
+          {% if item.below %}
+            {{ book_tree.book_links(item.below, attributes, menu_level + 1) }}
+          {% endif %}
+        </li>
+      {% endfor %}
+    </ul>
+  {% endif %}
+{% endmacro %}

core/themes/classy/templates/navigation/book-tree.html.twig

@@ -1,14 +1,42 @@
 {#
 /**
  * @file
- * Theme override for a book tree.
+ * Theme override to display a book tree.
  *
  * Returns HTML for a wrapper for a book sub-tree.
  *
  * Available variables:
- *   - tree: An HTML string containing the tree's items.
+ * - items: A nested list of book items. Each book item contains:
- *
+ *   - attributes: HTML attributes for the book item.
- * @see template_preprocess_book_tree()
+ *   - below: The book item child items.
+ *   - title: The book link title.
+ *   - url: The book link URL, instance of \Drupal\Core\Url.
  */
 #}
-<ul class="menu">{{ tree }}</ul>
+{% import _self as book_tree %}
+
+{#
+  We call a macro which calls itself to render the full tree.
+  @see http://twig.sensiolabs.org/doc/tags/macro.html
+#}
+{{ book_tree.book_links(items, attributes, 0) }}
+
+{% macro book_links(items, attributes, menu_level) %}
+  {% import _self as book_tree %}
+  {% if items %}
+    {% if menu_level == 0 %}
+      <ul{{ attributes.addClass('menu') }}>
+    {% else %}
+      <ul class="menu">
+    {% endif %}
+      {% for item in items %}
+        <li{{ item.attributes }}>
+          {{ link(item.title, item.url) }}
+          {% if item.below %}
+            {{ book_tree.book_links(item.below, attributes, menu_level + 1) }}
+          {% endif %}
+        </li>
+      {% endfor %}
+    </ul>
+  {% endif %}
+{% endmacro %}