Replace links.html.twig with item-list--links.html.twig

(not worth changing status, but if you reroll, there's a comment in there with no ending period.)

+1. This would be very, very useful. At the very least, a conversion function would be tremendously helpful -- i.e., something that can convert the $links normally sent to theme_links() into something theme_item_list() can understand.

Here's my initial stab at at converting links to an item list:

function links_to_item_list($links, $title = NULL, $type = 'ul', $attributes = NULL) {
  $items = array();

  foreach ($links as $link) {
    if (isset($link['href'])) {
      // Pass in $link as $options, they share the same keys.
      $items[] = l($link['title'], $link['href'], $link);
    }
    else if (!empty($link['title'])) {
      // Some links are actually not links, but we wrap these in <span> for adding title and class attributes
      if (empty($link['html'])) {
        $link['title'] = check_plain($link['title']);
      }
      $span_attributes = '';
      if (isset($link['attributes'])) {
        $span_attributes = drupal_attributes($link['attributes']);
      }
      $items[] = '<span'. $span_attributes .'>'. $link['title'] .'</span>';
    }
  }

  return theme('item_list', $items, $title, $type, $attributes);
}

Not sure whether this still makes sense when considering HTML5's <navigation> element? (http://www.alistapart.com/articles/previewofhtml5)

Yes plz!!! :)

There's a non-obvious difference between links and item_list: links use the passed-in array keys as class names for the LIs.

Adding tags from closed duplicate #1777326: Replace theme_links() with theme_item_list().

Copying over an essential comment from #1777326: Replace theme_links() with theme_item_list():

Hm. Closely related: #1300744: Introduce #type 'links'

I agree that it just creates a list, but there is actually quite some preprocessing involved.

My main goal of aforementioned issue was/is to retain the raw data of links in a list of link as long as possible, so modules are able to adjust the links prior to rendering (which is not easily possible currently).

I also wanted to get proper #access handling in there, so e.g., a theme would be able to output an inactive placeholder element for a link that the current user is not able to access (which is a typical feature request of designers/product/usability folks, so as to "hint" not yet registered users to what functionality they're missing by not being registered and so on -- whereas the theme/design wants to output the links/placeholder in the identical position).

However, in the end, I think what I'm aiming for is render array support for individual items in an item list. — That's a slightly weird but important difference between theme_links() and theme_item_list() right now, because only the former allows you to specify the link list items as arrays (so the link text + href + attributes are properly retained and only rendered into a string when the links are actually rendered).

Another related issue: #891112: Replace theme_item_list()'s 'data' items with render elements

updated summary

link

link

Well one thing, is that now we are going to need to update several automated tests to compare the rendered links with <div class="item-list">. That div is present in theme_item_list but I suppose not present in theme_links. Is that okay?

@Jen, I have updated 3 automated tests to include the wrapper div that theme_item_list adds. I bet that will fix some of these fails.

Working alongside dpintats #1842140: Remove title and wrapper div from item-list.html.twig

Oh okay, well then my patch #20 doesn't make sense if the wrapper markup is being removed.

#23: Yes.

Besides the issue with this wrapper (which I think we can safely keep in the test until the other issue removes the wrapper), @johnnydarkko and I found out the cause of [all?] the failing tests--the missing key class on list items previously set by theme_links in theme.inc:1723:

      $class = array();
      // Use the array key as class name.
      $class[] = drupal_html_class($key);

Expecting we want to maintain this key class, the solution to the issue is to implement a preprocess function for adding the class for links lists:

function template_preprocess_item_list__links()

And of course this preprocess won't get picked up until one or both of these issues are resolved:

#939462: Specific preprocess functions for theme hook suggestions are not invoked and/or #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

In the interim, I believe we should be able to get the preprocess function firing by explicitly adding the suggestion to drupal_common_theme():

    'item_list__links' => array(
      'variables' => array('items' => array(), 'list_type' => 'ul', 'attributes' => array()),
      'preprocess functions' => array('template_preprocess_item_list__links'),
      'base hook' => 'item_list',
    ),

So, in summary, to move forward on this, I think:

1. Leave the wrapper in tests until #1842140: Remove title and wrapper div from item-list.html.twig is fixed and/or RTBC.
2. Implement preprocess for item_list__links to add key class back to each item in 'items'.

#1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter() is now in core.

Once #2035055: Introduce hook_theme_prepare[_alter]() and remove hook_preprocess_HOOK() makes it in, hook_theme_prepare_item_list__links() can become a reality.

In reality though, we should simply replace any calls to theme('links') or '#theme' => 'links' with simply item_list__links and just remove this completely unnecessary theme implementation. We'd still keep the suggestion prepare hook though.

twig

With #1939064: Convert theme_links() to Twig in head, theme_links() no longer exists. How is this issue affected?

We now have two templates: links.html.twig and item-list.html.twig and although they both produce very similar HTML code, the "template logic" in them is quite different so I would be inclined to either:

Option A:

Close this as works as designed.

Option B:

Eat our own dog food: remove links.html.twig and implement a real template suggestion in core: item-list--links.html.twig.

We need to account for all of the differences from links.html.twig including:

• Add the ability to define heading level, attributes and text. Note: heading text no longer makes a lot of sense in the context of an item-list. The wrapper div and heading has already been removed from item-list.html.twig and this could be enough of an argument to go for option A above and leave these two templates as-is.
• Add the ability to have keyed items (and turn those keys into classes).

This really postponed on this related issue, ultimately. Now that it's in, "Option B" is actually a reality and the original intent of this issue. It still needs to happen... at some point.

However, given how close to RC we are, maybe it should be postponed for 8.1.x or 9.x? There is not really a good/current policy towards BC breakages for themes/templates/CSS in core.

I identified a couple questions/clarifications while cleaning up the issue summary. The other issue #1842140: Remove title and wrapper div from item-list.html.twig states:

Call theme('links') when you need a wrapper div and/or a title.
Call theme(item_list) when you only need a list.

However, a wrapper div and title have nothing to do with whether a list has links or not, so this could certainly be improved. Assuming that issue is not blocked/postponed and will get in before this one, then we should really figure out and discuss a strategy here that improves/clarifies DX and TX at the same time.

1. We should improve the template so wrapper/title is independent of links. For this, I can see two options (don't know which is better, but am leaning towards the latter):

• Add the "list with wrapper div and title" functionality to the general item-list.html.twig template. By default theme('list_items') would not print the wrapper/title. Then devs could call theme('list_items__wrapped') and we specify in the preprocess to use 'div' as the wrapper tag, and 'h2' as the title tag.

OR

• Leave list-items.html.twig as is and implement the list-items--wrapped.html.twig that adds the wrapper and title functionality and then imports (or uses twig macro) to leverage existing link-list.html.twig code for the list rendering.

2. A list of links is semantically-speaking, just "navigation". So we could use semantic <nav> wrapper and <hX> for title tag, with aria landmarks (a la pager.html.twig) and then either leverage link-list--wrapped.html.twig or implement another template link-list--navigation.html.twig that hardcodes these tags into a template that a themer could easily target and override if desired.

links.html.twig is a part of stable theme

Moving back to 8.2.x

We can't actually remove theme hooks/templates in minor releases, but we can remove usages and deprecate them.

There's a patch here.

it still blocked on #1842140: Remove title and wrapper div from item-list.html.twig because markup between core/modules/system/templates/links.html.twig and core/modules/system/templates/item-list.html.twig is not possible to make equal