Move links classes from preprocess to templates

Link keys used as classes moved to twig template.
I did not touch the header class code (below) in template_preprocess_links , as I think it will be removed in #2285493: Remove deprecated 'class' property from #theme 'links' and #theme 'menu_tree' heading arrays.

      // @todo Remove backwards compatibility for $heading['class'].
      if (isset($heading['class'])) {
        $heading['attributes']['class'] = $heading['class'];
      }

+++ b/core/includes/theme.inc
@@ -1047,6 +1043,7 @@ function template_preprocess_links(&$variables) {
+      $li_attributes = array();

I'm not clear what this is doing?

It just moves $li_attributes = array(); approx. 15 lines down. This is the resulting code:

   foreach ($links as $key => $link) {
      $item = array('key' => $key);
      $link += array(
        'href' => NULL,
        'route_name' => NULL,
        'route_parameters' => NULL,
        'ajax' => NULL,
      );

      $keys = array('title', 'href', 'route_name', 'route_parameters');
      $link_element = array(
        '#type' => 'link',
        '#title' => $link['title'],
        '#options' => array_diff_key($link, array_combine($keys, $keys)),
        '#href' => $link['href'],
        '#route_name' => $link['route_name'],
        '#route_parameters' => $link['route_parameters'],
        '#ajax' => $link['ajax'],
      );

      // Handle links and ensure that the active class is added on the LIs, but
      // only if the 'set_active_class' option is not empty.
      $li_attributes = array();
      if (isset($link['href']) || isset($link['route_name'])) {
        if (!empty($variables['set_active_class'])) {

          // Also enable set_active_class for the contained link.
          $link_element['#options']['set_active_class'] = TRUE;

          if (!empty($link['language'])) {
            $li_attributes['hreflang'] = $link['language']->id;
          }

[edit: move verbose explanation]

Back to needs review.

Made the necessary test updates, this should be green now.

This is looking good to me now, needs more reviews though.

Tested this manually and classes seems to be the same after applying the patch!

Reroll

putting back

+++ b/core/includes/theme.inc
@@ -975,7 +975,7 @@ function template_preprocess_links(&$variables) {
+      $item = array('key' => $key);

+++ b/core/modules/system/templates/links.html.twig
@@ -15,6 +15,7 @@
+ *   - key: The link key. To be used as link class.

@@ -45,7 +46,7 @@
+      <li{{ item.attributes.addClass(item.key|clean_class) }}>

Key seems to be a really nondescriptive name. Let's call it css class since the docs of template_preprocess_links() says The key for each link is used as its CSS class

Rerolled and fixed #18

+++ b/core/includes/theme.inc
@@ -978,16 +978,13 @@ function template_preprocess_links(&$variables) {
+      $item = array('css_class' => $key);

+++ b/core/modules/system/templates/links.html.twig
@@ -15,6 +15,7 @@
+ *   - css_class: The link CSS class.

@@ -45,7 +46,7 @@
+      <li{{ item.attributes.addClass(item.css_class|clean_class) }}>

I think we should change the variable name to link key to say what it is rather than what it is used for. And that way doing the clean_class in twig and not using drupal_html_class in the preprocess function makes sense.

Changed variable name as suggested from css_class to link_key.

Sending to testbot :)

Fixes #21

+++ b/core/includes/theme.inc
@@ -978,16 +978,13 @@ function template_preprocess_links(&$variables) {
-      $item = array();
+      $item = array('link_key' => $key);

I think I've missed a trick here. There is no need for this extra variable. We just shouldn't remove the keys from $variables['links']. So

      // Add the item to the list of links.
      $variables['links'][] = $item;

becomes

      // Add the item to the list of links.
      $variables['links'][$key] = $item;

Then we can use the key in the twig template.

Maybe we finally could get this in ;)

Committed/pushed to 8.0.x, thanks!