Rename 'type' variable of theme_item_list() to 'list_type'

Patch re-rolled. Uploading to check for for greenlight.

I don't like the variable name 'tag' since there are actually a lot of HTML tags in the item_list Twig template. Tag really only makes sense if you are concatenating strings to generate HTML tags as we did in Drupal 7: $output .= '<' . $tag . new Attribute($list_attributes) . '>';.

But we are removing this kind of behavior from Drupal 8, so I think it's important that we keep the names of variables closer to what they mean. In this case 'type' is indicating what type of list we were generating. I had changed the variable name to 'list_type' in the Twig template. I find that slightly more sensical than just 'tag'.

Here's an example of how the twig template would read using tag:

{% if items %}
<div class="item-list">
  {% if title is not empty -%}
    <h3>{{ title }}</h3>
  {% endif -%}
  {% if tag is ul %}
    <ul {{ attributes }}>
  {% else %}
    <ol {{ attributes }}>
  {% endif %}
  ...
{% endif %}

The above just doesn't read nicely to me. If we used list_type instead it makes a little more sense when you look at it:

{% if items %}
<div class="item-list">
  {% if title is not empty -%}
    <h3>{{ title }}</h3>
  {% endif -%}
  {% if list_type is ul %}
    <ul {{ attributes }}>
  {% else %}
    <ol {{ attributes }}>
  {% endif %}
    ...
{% endif %}

Updating issue title. Rerolled patch with alternate variable name.

#10: 1828536-drupal8-theme-item-list-type-tag.10.patch queued for re-testing.

retesting previous patch to confirm that I broke everything :/

I think I found the problem. One of the changes to "tag" wasn't changed to "list_type."

After applying patch 1828536-16-list-type.patch in #16 from dcam, it appears that list items can be made/viewed with no errors using the alternate variable name. See screenshot.

Looks very good to me, passes tests, had manual testing. => RTBC

Missing changes to
core/modules/tour/lib/Drupal/tour/TourRenderController.php
core/modules/views/views.theme.inc

Related #1300744: Introduce #type 'links'

not tag

not sure this should still be assigned to sun. I see a few others rolling patches here.

added blocking issue

also related #2005970: In renderable arrays, #type should provide a #theme suggestion, similar to how drupal_prepare_form() works.

I just double checked the changes, looks good. Will need a change notification, tagging.

Minor coding standards nitpick, otherwise RTBC from me:

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -634,7 +634,7 @@ function render_items($items) {
+            'list_type' => $this->options['multi_type']

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -1698,7 +1698,7 @@ public function buildOptionsForm(&$form, &$form_state) {
+                  'list_type' => $type

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -873,7 +873,7 @@ public function buildOptionsForm(&$form, &$form_state) {
+                'list_type' => $type

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/PrerenderList.phpundefined
@@ -83,7 +83,7 @@ function render_items($items) {
+            'list_type' => $this->options['type']

These should all have a trailing comma per https://drupal.org/coding-standards#array.

#25: 1828536-25.patch queued for re-testing.

#25: 1828536-25.patch queued for re-testing.

Looks good to me!

Needs a reroll

curl https://drupal.org/files/1828536-25.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7273  100  7273    0     0   7424      0 --:--:-- --:--:-- --:--:--  9012
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php:1698
error: core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php:873
error: core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/field/PrerenderList.php:83
error: core/modules/views/lib/Drupal/views/Plugin/views/field/PrerenderList.php: patch does not apply
error: patch failed: core/modules/views/views.theme.inc:1185
error: core/modules/views/views.theme.inc: patch does not apply

re-rolled

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php
@@ -1695,12 +1695,11 @@ public function buildOptionsForm(&$form, &$form_state) {
-              $item_list = array(
-                '#theme' => 'item_list',
-                '#items' => $items,
-                '#type' => $type,
-              );
-              $output .= drupal_render($item_list);
+              $output .= theme('item_list',
+                array(
+                  'items' => $items,
+                  'list_type' => $type,
+                ));

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php
@@ -870,12 +870,11 @@ public function buildOptionsForm(&$form, &$form_state) {
-            $item_list = array(
-              '#theme' => 'item_list',
-              '#items' => $items,
-              '#type' => $type,
-            );
-            $output .= drupal_render($item_list);
+            $output .= theme('item_list',
+              array(
+                'items' => $items,
+                'list_type' => $type,

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/PrerenderList.php
@@ -79,13 +79,12 @@ function render_items($items) {
-        $item_list = array(
-          '#theme' => 'item_list',
-          '#items' => $items,
-          '#title' => NULL,
-          '#type' => $this->options['type'],
-        );
-        return drupal_render($item_list);
+        return theme('item_list',
+          array(
+            'items' => $items,
+            'title' => NULL,
+            'list_type' => $this->options['type'],

+++ b/core/modules/views/views.theme.inc
@@ -1185,16 +1185,14 @@ function theme_views_mini_pager($vars) {
-  $item_list = array(
-    '#theme' => 'item_list',
-    '#items' => $items,
-    '#title' => NULL,
-    '#type' => 'ul',
-    '#attributes' => array(
-      'class' => array('pager'),
-    ),
+  return theme('item_list',
+    array(
+      'items' => $items,
+      'title' => NULL,
+      'list_type' => 'ul',
+      'attributes' => array('class' => array('pager')),
...
-  return drupal_render($item_list);

These look like merge errors. Should be simply

- '#type' => $type,
+ '#list_type' => $type,

for each of them

Yeah, #25 needs a re-re-roll without re-introducing theme() everywhere.

ok, thanks for the quick feedback! Hopefully this one is correct.

Yup, that's it. Awesome response time!!! :-) Let's do it.

Needs another quick reroll after #2010672: Rename 'type' variable of theme_mark to 'status' was committed. Thanks for all the rerolls @benjf!

re-rolling now

re-rolled to latest head

#41 passed and ready for review or rework

The patch looks fine to me, but the interdiff is backwards I think.

I've scanned the codebase for all usages of the item_list theme and everything appears to be updated correctly. This needs a change notice...

Committed 088b191 and pushed to 8.x. Thanks!

is a task

Created a small change notification similar to the one for #2010672: Rename 'type' variable of theme_mark to 'status':

https://drupal.org/node/2030833