Issue #1939064 by joelpittet, pwieck, farrington, mark.labrecque, Cottser, InternetDevels, mdrummond, drupalninja99, BarisW, jenlampton: Convert theme_links() to Twig.

core/includes/common.inc

@@ -3414,7 +3414,7 @@ function drupal_pre_render_link($element) {
  * Pre-render callback: Collects child links into a single array.
  *
  * This function can be added as a pre_render callback for a renderable array,
- * usually one which will be themed by theme_links(). It iterates through all
+ * usually one which will be themed by links.html.twig. It iterates through all
  * unrendered children of the element, collects any #links properties it finds,
  * merges them into the parent element's #links array, and prevents those
  * children from being rendered separately.
@@ -3435,21 +3435,21 @@ function drupal_pre_render_link($element) {
  *     '#theme' => 'links__node__comment',
  *     '#links' => array(
  *       // An array of links associated with node comments, suitable for
- *       // passing in to theme_links().
+ *       // passing in to links.html.twig.
  *     ),
  *   ),
  *   'statistics' => array(
  *     '#theme' => 'links__node__statistics',
  *     '#links' => array(
  *       // An array of links associated with node statistics, suitable for
- *       // passing in to theme_links().
+ *       // passing in to links.html.twig.
  *     ),
  *   ),
  *   'translation' => array(
  *     '#theme' => 'links__node__translation',
  *     '#links' => array(
  *       // An array of links associated with node translation, suitable for
- *       // passing in to theme_links().
+ *       // passing in to links.html.twig.
  *     ),
  *   ),
  * );

core/includes/form.inc

@@ -1390,7 +1390,7 @@ function form_pre_render_actions_dropbutton(array $element) {
       }
       // Add this button to the corresponding dropbutton.
       // @todo Change #type 'dropbutton' to be based on theme_item_list()
-      //   instead of theme_links() to avoid this preemptive rendering.
+      //   instead of links.html.twig to avoid this preemptive rendering.
       $button = drupal_render($element[$key]);
       $dropbuttons[$dropbutton]['#links'][$key] = array(
         'title' => $button,

core/includes/menu.inc

@@ -1837,7 +1837,7 @@ function menu_navigation_links($menu_name, $level = 0) {
       if ($item['link']['href'] == $router_item['tab_root_href'] && $item['link']['href'] != current_path()) {
         $l['attributes']['class'][] = 'active';
       }
-      // Keyed with the unique mlid to generate classes in theme_links().
+      // Keyed with the unique mlid to generate classes in links.html.twig.
       $links['menu-' . $item['link']['mlid'] . $class] = $l;
     }
   }

core/includes/theme.inc

@@ -1168,9 +1168,11 @@ function template_preprocess_status_messages(&$variables) {
 }
 
 /**
- * Returns HTML for a set of links.
+ * Prepares variables for links templates.
  *
- * @param $variables
+ * Default template: links.html.twig.
+ *
+ * @param array $variables
  *   An associative array containing:
  *   - links: An associative array of links to be themed. The key for each link
  *     is used as its CSS class. Each link should be itself an array, with the
@@ -1194,13 +1196,12 @@ function template_preprocess_status_messages(&$variables) {
  *     l() as its $options parameter.
  *   - attributes: A keyed array of attributes for the UL containing the
  *     list of links.
- *   - set_active_class: (optional) Whether theme_links() should compare the
+ *   - set_active_class: (optional) Whether each link should compare the
  *     route_name + route_parameters or href (path), language and query options
- *     to the current URL for each of the links, to determine whether the link
- *     is "active". If so, an "active" class will be applied to the list item
- *     containing the link, as well as the link itself. It is important to use
- *     this sparingly since it is usually unnecessary and requires extra
- *     processing.
+ *     to the current URL, to determine whether the link is "active". If so, an
+ *     "active" class will be applied to the list item containing the link, as
+ *     well as the link itself. It is important to use this sparingly since it
+ *     is usually unnecessary and requires extra processing.
  *     For anonymous users, the "active" class will be calculated on the server,
  *     because most sites serve each anonymous user the same cached page anyway.
  *     For authenticated users, the "active" class will be calculated on the
@@ -1223,8 +1224,8 @@ function template_preprocess_status_messages(&$variables) {
  *     http://juicystudio.com/article/screen-readers-display-none.php and
  *     http://www.w3.org/TR/WCAG-TECHS/H42.html for more information.
  *
- * theme_links() unfortunately duplicates the "active" class handling of l() and
- * LinkGenerator::generate() because it needs to be able to set the "active"
+ * Unfortunately links templates duplicate the "active" class handling of l()
+ * and LinkGenerator::generate() because it needs to be able to set the "active"
  * class not on the links themselves ("a" tags), but on the list items ("li"
  * tags) that contain the links. This is necessary for CSS to be able to style
  * list items differently when the link is active, since CSS does not yet allow
@@ -1236,11 +1237,9 @@ function template_preprocess_status_messages(&$variables) {
  * @see \Drupal\Core\Utility\LinkGenerator::generate()
  * @see system_page_build()
  */
-function theme_links($variables) {
+function template_preprocess_links(&$variables) {
   $links = $variables['links'];
-  $attributes = $variables['attributes'];
-  $heading = $variables['heading'];
-  $output = '';
+  $heading = &$variables['heading'];
 
   if (!empty($links)) {
     // Prepend the heading to the list, if any.
@@ -1258,15 +1257,14 @@ function theme_links($variables) {
       if (isset($heading['class'])) {
         $heading['attributes']['class'] = $heading['class'];
       }
-
-      $output .= '<' . $heading['level'] . new Attribute($heading['attributes']) . '>';
-      $output .= String::checkPlain($heading['text']);
-      $output .= '</' . $heading['level'] . '>';
+      // Convert the attributes array into an Attribute object.
+      $heading['attributes'] = new Attribute($heading['attributes']);
+      $heading['text'] = String::checkPlain($heading['text']);
     }
 
-    $output .= '<ul' . new Attribute($attributes) . '>';
-
+    $variables['links'] = array();
     foreach ($links as $key => $link) {
+      $item = array();
       $link += array(
         'href' => NULL,
         'route_name' => NULL,
@@ -1320,29 +1318,23 @@ function theme_links($variables) {
           $li_attributes['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager.cached')->getSystemPath($path);
         }
 
-        $item = drupal_render($link_element);
+        $item['link'] = $link_element;
       }
+
       // Handle title-only text items.
-      else {
-        // Merge in default array properties into $link.
-        $link += array(
-          'html' => FALSE,
-        );
-        $item = ($link['html'] ? $link['title'] : String::checkPlain($link['title']));
-        if (isset($link['attributes'])) {
-          $item = '<span' . new Attribute($link['attributes']) . '>' . $item . '</span>';
-        }
+      $text = (!empty($link['html']) ? $link['title'] : String::checkPlain($link['title']));
+      $item['text'] = $text;
+      if (isset($link['attributes'])) {
+        $item['text_attributes'] = new Attribute($link['attributes']);
       }
 
-      $output .= '<li' . new Attribute($li_attributes) . '>';
-      $output .= $item;
-      $output .= '</li>';
+      // Handle list item attributes.
+      $item['attributes'] = new Attribute($li_attributes);
+
+      // Add the item to the list of links.
+      $variables['links'][] = $item;
     }
-
-    $output .= '</ul>';
   }
-
-  return $output;
 }
 
 /**
@@ -2579,6 +2571,7 @@ function drupal_common_theme() {
     ),
     'links' => array(
       'variables' => array('links' => array(), 'attributes' => array('class' => array('links')), 'heading' => array(), 'set_active_class' => FALSE),
+      'template' => 'links',
     ),
     'dropbutton_wrapper' => array(
       'variables' => array('children' => NULL),

core/lib/Drupal/Core/Menu/ContextualLinkManagerInterface.php

@@ -24,7 +24,7 @@ interface ContextualLinkManagerInterface {
   public function getContextualLinkPluginsByGroup($group_name);
 
   /**
-   * Gets the contextual links prepared as expected by theme_links().
+   * Gets the contextual links prepared as expected by links.html.twig.
    *
    * @param string $group_name
    *   The group name.

core/modules/contextual/contextual.module

@@ -271,7 +271,7 @@ function contextual_pre_render_links($element) {
     $items += $contextual_links_manager->getContextualLinksArrayByGroup($group, $args['route_parameters'], $args['metadata']);
   }
 
-  // Transform contextual links into parameters suitable for theme_links().
+  // Transform contextual links into parameters suitable for links.html.twig.
   $links = array();
   foreach ($items as $class => $item) {
     $class = drupal_html_class($class);

core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php

@@ -2869,7 +2869,7 @@ abstract class WebTestBase extends TestBase {
    * Asserts themed output.
    *
    * @param $callback
-   *   The name of the theme function to invoke; e.g. 'links' for theme_links().
+   *   The name of the theme hook to invoke; e.g. 'links' for links.html.twig.
    * @param $variables
    *   An array of variables to pass to the theme function.
    * @param $expected

core/modules/system/css/system.theme.css

@@ -398,7 +398,7 @@ ul.menu a.active {
 }
 
 /**
- * Markup generated by theme_links().
+ * Markup generated by links.html.twig.
  */
 ul.inline,
 ul.links.inline {

core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php

@@ -149,9 +149,13 @@ class FunctionsTest extends WebTestBase {
   }
 
   /**
-   * Tests theme_links().
+   * Tests links.html.twig.
    */
   function testLinks() {
+    // Turn off the query for the l() function to compare the active
+    // link correctly.
+    $original_query = \Drupal::request()->query->all();
+    \Drupal::request()->query->replace(array());
     // Verify that empty variables produce no output.
     $variables = array();
     $expected = '';
@@ -198,6 +202,9 @@ class FunctionsTest extends WebTestBase {
     $expected = $expected_heading . $expected_links;
     $this->assertThemeOutput('links', $variables, $expected);
 
+    // Restore the original request's query.
+    \Drupal::request()->query->replace($original_query);
+
     // Verify that passing an array as heading works (core support).
     $variables['heading'] = array('text' => 'Links heading', 'level' => 'h3', 'class' => 'heading');
     $expected_heading = '<h3 class="heading">Links heading</h3>';

core/modules/system/system.module

@@ -2359,7 +2359,7 @@ function system_page_build(&$page) {
   // - applying a post-render cache callback if the current user is anonymous.
   // @see l()
   // @see \Drupal\Core\Utility\LinkGenerator::generate()
-  // @see theme_links()
+  // @see template_preprocess_links()
   // @see \Drupal\system\Controller\SystemController::setLinkActiveClass
   if (\Drupal::currentUser()->isAuthenticated()) {
     $page['#attached']['library'][] = array('system', 'drupal.active-link');

core/modules/system/templates/links.html.twig

@@ -0,0 +1,59 @@
+{#
+/**
+ * @file
+ * Default theme implementation for a set of links.
+ *
+ * Available variables:
+ * - attributes: Attributes for the UL containing the list of links.
+ * - links: Links to be output.
+ *   Each link will have the following elements:
+ *   - title: The link text.
+ *   - href: The link URL. If omitted, the 'title' is shown as a plain text
+ *     item in the links list. If 'href' is supplied, the entire link is passed
+ *     to l() as its $options parameter.
+ *   - html: (optional) Whether or not 'title' is HTML. If set, the title will
+ *     not be passed through check_plain().
+ *   - attributes: (optional) HTML attributes for the anchor, or for the <span>
+ *     tag if no 'href' is supplied.
+ * - heading: (optional) A heading to precede the links.
+ *   - text: The heading text.
+ *   - level: The heading level (e.g. 'h2', 'h3').
+ *   - attributes: (optional) A keyed list of attributes for the heading.
+ *   If the heading is a string, it will be used as the text of the heading and
+ *   the level will default to 'h2'.
+ *
+ *   Headings should be used on navigation menus and any list of links that
+ *   consistently appears on multiple pages. To make the heading invisible use
+ *   the 'visually-hidden' CSS class. Do not use 'display:none', which
+ *   removes it from screen-readers and assistive technology. Headings allow
+ *   screen-reader and keyboard only users to navigate to or skip the links.
+ *   See http://juicystudio.com/article/screen-readers-display-none.php and
+ *   http://www.w3.org/TR/WCAG-TECHS/H42.html for more information.
+ *
+ * @see template_preprocess_links()
+ *
+ * @ingroup themeable
+ */
+#}
+{% if links -%}
+  {%- if heading -%}
+    {%- if heading.level -%}
+      <{{ heading.level }}{{ heading.attributes }}>{{ heading.text }}</{{ heading.level }}>
+    {%- else -%}
+      <h2{{ heading.attributes }}>{{ heading.text }}</h2>
+    {%- endif -%}
+  {%- endif -%}
+  <ul{{ attributes }}>
+    {%- for item in links -%}
+      <li{{ item.attributes }}>
+        {%- if item.link -%}
+          {{ item.link }}
+        {%- elseif item.text_attributes -%}
+          <span{{ item.text_attributes }}>{{ item.text }}</span>
+        {%- else -%}
+          {{ item.text }}
+        {%- endif -%}
+      </li>
+    {%- endfor -%}
+  </ul>
+{%- endif %}

core/modules/system/tests/modules/ajax_test/lib/Drupal/ajax_test/Controller/AjaxTestController.php

@@ -63,7 +63,7 @@ class AjaxTestController {
       ),
     );
 
-    // Dialog behavior applied to links rendered by theme_links().
+    // Dialog behavior applied to links rendered by links.html.twig.
     $build['links'] = array(
       '#theme' => 'links',
       '#links' => array(

core/modules/views_ui/lib/Drupal/views_ui/ViewEditFormController.php

@@ -957,7 +957,7 @@ class ViewEditFormController extends ViewFormControllerBase {
         break;
     }
 
-    // Create an array of actions to pass to theme_links
+    // Create an array of actions to pass to links template.
     $actions = array();
     $count_handlers = count($executable->display_handler->getHandlers($type));