Problem/Motivation

Convert theme_item_list() theme function to .html.twig template and template_preprocess_item_list() function for demonstration of Twig theming and ideal separation of concerns between logic and markup of item lists.

Remaining tasks

  1. Remove first/last/even/odd classes after fixing #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors.
  2. #1785310: Support custom child list types in theme_item_list()
  3. Decide what to do with <div class="item-list"></div> wrapper. See comment #32

User interface changes

None.

API changes

None at the moment; Remaining tasks may dictate that client code output wrapper element.

See theme_item_list().

CommentFileSizeAuthor
#26 d8tts-convert-theme-item-list-twig-1800726-26.patch3.85 KBsteveoliver
#25 d8tts-convert-theme-item-list-twig-1800726-25.patch3.59 KBsteveoliver
#19 d8tts-convert-theme-item-list-twig-1800726-19.patch3.91 KBsteveoliver
#18 d8tts-convert-theme-item-list-twig-1800726-18.patch3.94 KBsteveoliver
#16 d8tts-convert-theme-item-list-twig-1800726-16.patch4 KBsteveoliver
#15 d8tts-convert-theme-item-list-twig-1800726-15.patch3.92 KBsteveoliver
#14 twig-item-list-1800726-14.patch4.44 KBsteveoliver
#12 twig-item-list-1800726-11.patch3.88 KBsteveoliver
#10 twig-item-list-1800726-10.patch3.82 KBsteveoliver
#9 d8tts-convert-theme-item-list-twig-1800726-9.patch4.05 KBsteveoliver
#8 d8tts-convert-theme-item-list-twig-1800726-8.patch3.96 KBsteveoliver
#5 1800726-theme-item-list-bartik.png6.25 KBsteveoliver
#5 1800726-theme-item-list-stark.png7.17 KBsteveoliver
#5 d8tts-convert-theme-item-list-twig-1800726-5.patch2.88 KBsteveoliver
#2 d8tts-convert-theme-item-list-twig-1800726-1.patch1.55 KBsteveoliver
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented

Howto render a tree:

http://stackoverflow.com/questions/8326482/twig-how-to-render-a-tree

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Active » Needs review
FileSize
d8tts-convert-theme-item-list-twig-1800726-1.patch1.55 KB

Seems to be working...

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented 
+++ b/core/themes/stark/templates/system/item-list.twigundefined
@@ -0,0 +1,40 @@
+  <h3> {{ title }} </h3>

Should check if title is set

+++ b/core/themes/stark/templates/system/item-list.twigundefined
@@ -0,0 +1,40 @@
+      {% for child_items in item.children %}

This loop is not necessary.

General:

We need to move the attribute processing part to preprocess function.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented

Here is some code form initial code sprint that should help with the first, last, even and odd attributes so the code int the preprocess can be simpler.

{% if items %}
  <{{ type }} class="{{ items.attributes.class }}" {{ items.attributes }}>
  {% for item in items %}
    <li class="{{ item.attributes.class }} {{ cycle(['even', 'odd'], loop.index) }} {{ loop.first ? 'first' : '' }} {{ loop.last ? 'last' : '' }}" {{- item.attributes }}>
      {{- item -}}
    </li>
  {% endfor %}
  </{{ type }}>
{% endif %}
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Category: support » task
Status: Needs review » Needs work
FileSize
d8tts-convert-theme-item-list-twig-1800726-5.patch2.88 KB
1800726-theme-item-list-stark.png7.17 KB
1800726-theme-item-list-bartik.png6.25 KB

With this PHP code in a node:

$type = 'ol';
$title = 'Test Item List';
$attributes = array(
  'class' => 'top-level-list-class',
  'id' => 'top-level-list-id',
);
$items = array();
$items[] = 'top level item - 1';
$items[] = 'top level item - 2';
$items[] = array(
  'data' => 'top level item - 3',
  'children' => array(
    'second level item - 3.1',
    'second level item - 3.2',
    array(
      'data' => 'second level item - 3.3',
      'children' => array(
        'third level item - 3.3.1',
        'third level item - 3.3.2',
      ),
    ),
  ),
);
print theme('item_list', array(
  'items' => $items,
  'title' => $title,
  'type' => $type,
  'attributes' => $attributes
));

Bartik produces this output:

...with this markup:

<div class="item-list"><h3>Test Item List</h3><ol id="top-level-list-id" class="top-level-list-class"><li class="odd first">top level item - 1</li><li class="even">top level item - 2</li><li class="odd last">top level item - 3<div class="item-list"><ol><li class="odd first">second level item - 3.1</li><li class="even">second level item - 3.2</li><li class="odd last">second level item - 3.3<div class="item-list"><ol><li class="odd first">third level item - 3.3.1</li><li class="even last">third level item - 3.3.2</li></ol></div></li></ol></div></li></ol></div>

Stark, with the attached patch, produces this output:

...with this markup:

<div class="item-list">
      <h3>Test Item List</h3>
          <ol class="top-level-list-class">
          <li class="odd first">top level item - 1      </li>
          <li class="even">top level item - 2      </li>
          <li class="odd last">          <div class="item-list">
          <ol class="">
          <li class="odd first">second level item - 3.1      </li>
          <li class="even">second level item - 3.2      </li>
          <li class="odd last">          <div class="item-list">
          <ol class="">
          <li class="odd first">third level item - 3.3.1      </li>
          <li class="even last">third level item - 3.3.2      </li>
        </ol>
  </div>
              </li>
        </ol>
  </div>
              </li>
        </ol>
  </div>

Issues:

  1. Parent items (data) are not being rendered, while there children are.
  2. template_preprocess_item_list() isn't being called for either Bartik or Stark.
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

Also, in Stark, I'm getting the following errors:

User error: "class" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "id" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "data" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "0" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "1" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "data" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "0" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "1" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "data" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "0" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).
User error: "1" is an invalid render array key in element_children() (line 6251 of core/includes/common.inc).

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented

JFYI: I fixed #6 in front-end branch.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-8.patch3.96 KB

OK, I'm stuck here with children attributes. I expect things to work, don't get errors, but also don't get children attributes. Patch attached. Use this PHP code to test:

$type = 'ol';
$title = 'Test Item List';
$attributes = array(
  'class' => 'top-level-list-class',
  'id' => 'top-level-list-id',
);
$items = array();
$items[] = 'top level item - 1';
$items[] = 'top level item - 2';
$items[] = array(
  'class' => 'top-level-item-3-class',
  'id' => 'top-level-item-3-id',
  'data' => 'top level item - 3',
  'children' => array(
    'second level item - 3.1',
    'second level item - 3.2',
    array(
      'class' => 'second-level-item-3-3-class',
      'id' => 'second-level-item-3-3-id',
      'data' => 'second level item - 3.3',
      'children' => array(
        'third level item - 3.3.1',
        'third level item - 3.3.2',
      ),
    ),
  ),
);
print theme('item_list', array(
  'items' => $items,
  'title' => $title,
  'type' => $type,
  'attributes' => $attributes
));
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Needs work » Needs review
FileSize
d8tts-convert-theme-item-list-twig-1800726-9.patch4.05 KB

'class' needed to be an array. Test this code with the attached patch. Works for me. Notice comment at http://api.drupal.org/api/drupal/core%21includes%21theme.inc/function/th... for question about supporting children lists with different types than their parents.

$type = 'ol';
$title = 'Test Item List';
$attributes = array(
  'class' => 'top-level-list-class',
  'id' => 'top-level-list-id',
);
$items = array();
$items[] = 'top level item - 1';
$items[] = 'top level item - 2';
$items[] = array(
  'class' => array('top-level-item-3-class'),
  'id' => 'top-level-item-3-id',
  'data' => 'top level item - 3',
  'children' => array(
    'second level item - 3.1',
    'second level item - 3.2',
    array(
      'type' => 'ul', // see theme_item_list documentation question.
      'class' => array('second-level-item-3-3-class'),
      'id' => 'second-level-item-3-3-id',
      'data' => 'second level item - 3.3',
      'children' => array(
        'third level item - 3.3.1',
        'third level item - 3.3.2',
      ),
    ),
  ),
);
print theme('item_list', array(
  'items' => $items,
  'title' => $title,
  'type' => $type,
  'attributes' => $attributes
));
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
twig-item-list-1800726-10.patch3.82 KB

Answer to child type question in #9: NO for now.

Use this test code, with attached patch (has cleaned up preprocess functions):

$type = 'ol';
$title = 'Test Item List';
$attributes = array(
  'class' => 'top-level-list-class',
  'id' => 'top-level-list-id',
);
$items = array();
$items[] = 'top level item - 1';
$items[] = 'top level item - 2';
$items[] = array(
  'class' => array('top-level-item-3-class'),
  'id' => 'top-level-item-3-id',
  'data' => 'top level item - 3',
  'children' => array(
    'second level item - 3.1',
    'second level item - 3.2',
    array(
      'class' => array('second-level-item-3-3-class'),
      'id' => 'second-level-item-3-3-id',
      'data' => 'second level item - 3.3',
      'children' => array(
        'third level item - 3.3.1',
        'third level item - 3.3.2',
      ),
    ),
  ),
);
print theme('item_list', array(
  'items' => $items,
  'title' => $title,
  'type' => $type,
  'attributes' => $attributes
));
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Needs review » Needs work

Child item attributes still need work...

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Needs work » Needs review
FileSize
twig-item-list-1800726-11.patch3.88 KB

OK, child attributes work. Please review attached patch with this code:

$type = 'ol';
$title = 'Test Item List';
$attributes = array(
  'class' => 'top-level-list-class',
  'id' => 'top-level-list-id',
);
$items = array();
$items[] = 'top level item - 1';
$items[] = 'top level item - 2';
$items[] = array(
  'class' => array('top-level-item-3-class'),
  'id' => 'top-level-item-3-id',
  'data' => 'top level item - 3',
  'children' => array(
    'second level item - 3.1',
    'second level item - 3.2',
    array(
      'class' => array('second-level-item-3-3-class'),
      'test-attribute' => 'test-attribute-value',
      'id' => 'second-level-item-3-3-id',
      'data' => 'second level item - 3.3',
      'children' => array(
        'third level item - 3.3.1',
        'third level item - 3.3.2',
        array(
          'data' => 'third level item - 3.3.3',
          'class' => array('third-level-item-3-3-3-class'),
          'id' => 'third-level-item-3-3-3-id',
          'children' => array(
            'fourth level item - 3.3.3.1',
            'fourth level item - 3.3.3.2',
          ),
        ),
      ),
    ),
  ),
);
print theme('item_list', array(
  'items' => $items,
  'title' => $title,
  'type' => $type,
  'attributes' => $attributes
));

Expected output, should be the same in Garland:

<div class="item-list">
      <h3>Test Item List</h3>
              <ol class="top-level-list-class" id="top-level-list-id">
                            <li class=" odd first">top level item - 1      </li>
                            <li class=" even">top level item - 2      </li>
                            <li class="top-level-item-3-class odd last" id="top-level-item-3-id">top level item - 3                    <div class="item-list">
              <ol>
                            <li class=" odd first">second level item - 3.1      </li>
                            <li class=" even">second level item - 3.2      </li>
                            <li class="second-level-item-3-3-class odd last" test-attribute="test-attribute-value" id="second-level-item-3-3-id">second level item - 3.3                    <div class="item-list">
              <ol>
                            <li class=" odd first">third level item - 3.3.1      </li>
                            <li class=" even">third level item - 3.3.2      </li>
                            <li class="third-level-item-3-3-3-class odd last" id="third-level-item-3-3-3-id">third level item - 3.3.3                    <div class="item-list">
              <ol>
                            <li class=" odd first">fourth level item - 3.3.3.1      </li>
                            <li class=" even last">fourth level item - 3.3.3.2      </li>
        </ol>
  </div>
              </li>
        </ol>
  </div>
              </li>
        </ol>
  </div>
              </li>
        </ol>
  </div>

Two issues/questions:
1. Preceding whitespace in class attribute. WTF?
2. Should we be doing striping in preprocess or in .twig?

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Needs work

1. Preceding whitespace in class attribute. WTF?
2. Should we be doing striping in preprocess or in .twig?

+++ b/core/themes/stark/templates/system/item-list.twigundefined
@@ -0,0 +1,52 @@
+      <li class="{{- item.attributes.class -}} {{ cycle([' even', ' odd'], loop.index) -}} {{ loop.first ? ' first' : '' -}} {{ loop.last ? ' last' : '' }}" {{ item.attributes -}}>
+        {{- value -}}

That can't work " even", " odd" will always have bad whitespace ... ;-)

But I think with HTML5 initiative we will strip out all even/odd anyway and replace with n-th child CSS3 selectors.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Needs work » Needs review
FileSize
twig-item-list-1800726-14.patch4.44 KB

I knew those spaces were there, ;) I thought they'd need to be somewhere, but as this patch shows, it seems Twig will take care of the whitespace.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Title: Convert theme_item_list from phptemplate to twig » Convert theme_item_list to item-list.twig
FileSize
d8tts-convert-theme-item-list-twig-1800726-15.patch3.92 KB

Changes in this patch:

1. Removed status-messages preprocess function accidentally included in previous patch.
2. Clean up preprocess function(s).
3. Attributes are always set, even if empty. (As per Fabianx's comment in the select.twig issue.).

Notes on preprocess:

1. There is one top level preprocess function that runs items through _item() and _attributes() callbacks to deal with nested lists. I don't see a way to do this cleanly within the .twig file, without preprocess (to address comments by chx and stevector in #1696786: Integrate Twig into core: Implementation issue.

Notes on .twig template:

1. .twig file contains lots of comments -- it's a little ugly. @todo: remove them?
2. it supports child items to be of different type (ol, ul) than parent. them_item_list() didn't support this; child always inherited parent type. @todo: remove this?

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-16.patch4 KB

Don't use the patch in #15. Use this patch. All notes from #15 still apply.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented 
+++ b/core/themes/stark/templates/system/item-list.twig
@@ -0,0 +1,52 @@
+<div class="item-list">
...
+  {% if items %}

There should be no output at all if there are no items.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-18.patch3.94 KB

But of course!

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-19.patch3.91 KB 
+++ b/core/themes/stark/templates/system/item-list.twig
@@ -0,0 +1,50 @@
+ *   Any other key/value pairs are used as HTML attributes for the list item
+ *   in 'data'.

"Any other key-value pairs" are not available in .twig; They are unset after being converted into attributes in _preprocess.

Patch attached includes comment reflecting this.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

The core issue about removing the (with CSS3) extraneous classes is here: #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

Thanks. What do you think, add a @todo until that lands?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

That would probably make sense. I just looked it up because I somehow had thought that was already in and was confused... :-)

criz’s picture

criz
he/him
Location Vienna/Austria
CreditAttribution: criz commented
Issue tags: +Twig, +theme system cleanup, +Theme Component Library

IMHO not only the even/odd/last/first-classes should be eliminated, also the hardcoded wrapping div should be stripped out in the basic implementation of item-list.twig.

A similar comment is here: http://api.drupal.org/api/drupal/includes!theme.inc/function/theme_item_...

I think this makes also sense when item-list.twig will be used in other templates like for pagers or menus. See #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template

Themes/modules can introduce this wrappers if they really need it easily.

jenlampton’s picture

jenlampton
she / her
CreditAttribution: jenlampton commented

I agree with @criz that we should remvoe the wrapper div from item list and only the list should be printed. the wrapper can also be added in templates calling item_list if needed.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-25.patch3.59 KB

Definitely. Here's the latest, also without all the verbose comments in .twig.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
FileSize
d8tts-convert-theme-item-list-twig-1800726-26.patch3.85 KB

Here, keeping classes for now (with @todo remove), but without wrapper div.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Not sure about removing the outer class. If we wouldn't support the list title, I would totally agree, because then we would have a straight mapping: theme_item_list() <-> ul/ol. Now the title seems somewhat misplaced without any outer nesting.

Either way, if we decide to remove the outer div, I think the outer if statement is superfluous, no?

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

Re: wrapper:
It seems like we have a decision to make between trade-offs.
1. With an item-list wrapper, each level of the list is wrapped (eww), but title is "grouped" with list (ok for top level, not really necessary for lower levels).
2. Without an item-list wrapper, title can seem a little misplaced, and it's the client code's responsibility to provide a wrapper (not a big deal, maybe preferred), but you don't have unnecessary wrappers around child lists.

Re: outer if... I wondered the same thing. But can't an empty item list conceivably be sent to the theme layer?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

But can't an empty item list conceivably be sent to the theme layer?

It can (although, conceivably, that could be considered a bug), but then the for loop will simply loop over an empty set and nothing will happen (unless you set a title, in which case that will get set).
Disclaimer: I don't know a thing about Twig, so it might be that Twig actually throws notices or something, but I think that is how it *should* work.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

I guess it would be nice if we could feasibly remove the title "feature" from theme_item_list() and let people print that on their own. That would definitely be a follow-up/own issue, though. Then I'd be more comfortable with removing the wrapper. I will definitely not stand in the way of it here, though. As you point out, nested lists with a bunch of divs in the middle, really are sucky...

jenlampton’s picture

jenlampton
she / her
CreditAttribution: jenlampton commented
Status: Needs review » Closed (fixed)

I have another template that needs to call item-list so I committed it to front end.

I renamed it to item-list.html.twig
I moved it into the theme.inc directory
and I also removed the even/odd first/last classes - but did this in a separate commit incase we need to revert :)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Closed (fixed) » Needs work

Removing the wrapper presents a serious regression. You cannot apply styles to the list heading anymore.

Crell’s picture

Crell CreditAttribution: Crell commented

sun: Whether it's a regression or a "dear god finally!" is up to the themers to decide, not backend devs like you and me.

IMO sane handling of an empty list is a requirement, because that simplifies code elsewhere. For a wrapping div (or some other block element), you and I don't have an opinion.

jenlampton’s picture

jenlampton
she / her
CreditAttribution: jenlampton commented
Status: Needs work » Closed (fixed)

Just a note that I added the wrapper div back in with a @todo to remove later.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Closed (fixed) » Needs work

Why not do <h3 class="item-list-title">?

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Needs work » Closed (fixed)

didn't mean to change status.. suggestion/idea for later.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Issue summary: View changes

Fixing code block.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Status: Closed (fixed) » Postponed

Added issue summary.

Note that current code allows children lists to be of different type than parent (i.e ol > li > ul), while old theme_item_list() would always pass the parent list type down to each child. Is this a feature we want to keep? I think so.

Gonna mark postponed until #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors allows us to remove the special classes.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Issue summary: View changes

Summarize issue.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Issue summary: View changes

Add a link to comment #32.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

Added #1785310: Support custom child list types in theme_item_list() to issue summary.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
  1. Added missing end quote to <li class=""> attribute (in ba7dde6b).
  2. Not printing {{ attributes.class }} before the rest of {{ attributes }} in d78b9c98.
steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

1. d2af331: Added support for child list type in line with theme_item_list() supports custom list types for child elements.

2. 547d374: Cleaned up output code whitespace:

Before:

<div class="item-list">       <h3>Test Item List</h3>
    <ol  class="top-level-list-class" id="top-level-list-id">
          <li >top level item - 1    </li>
          <li >top level item - 2    </li>
          <li  class="top-level-item-3-class" id="top-level-item-3-id">top level item - 3        <div class="item-list">     <ol >
          <li >second level item - 3.1    </li>
          <li >second level item - 3.2    </li>
          <li  class="second-level-item-3-3-class" test-attribute="test-attribute-value" id="second-level-item-3-3-id">second level item - 3.3        <div class="item-list">     <ol >
          <li >third level item - 3.3.1    </li>
          <li >third level item - 3.3.2    </li>
          <li  class="third-level-item-3-3-3-class" id="third-level-item-3-3-3-id">third level item - 3.3.3        <div class="item-list">     <ol >
          <li >fourth level item - 3.3.3.1    </li>
          <li >fourth level item - 3.3.3.2    </li>
    </ol>
</div>           </li>
    </ol>
</div>           </li>
    </ol>
</div>           </li>
    </ol>
</div>

After:

  <div class="item-list"> <h3>Test Item List</h3>
    <ol class="top-level-list-class" id="top-level-list-id">
      <li>top level item - 1</li>
      <li>top level item - 2</li>
      <li class="top-level-item-3-class" id="top-level-item-3-id">top level item - 3<div class="item-list">   <ol>
      <li>second level item - 3.1</li>
      <li>second level item - 3.2</li>
      <li class="second-level-item-3-3-class" test-attribute="test-attribute-value" id="second-level-item-3-3-id">second level item - 3.3<div class="item-list">   <ol>
      <li>third level item - 3.3.1</li>
      <li>third level item - 3.3.2</li>
      <li class="third-level-item-3-3-3-class" id="third-level-item-3-3-3-id">third level item - 3.3.3<div class="item-list">   <ol>
      <li>fourth level item - 3.3.3.1</li>
      <li>fourth level item - 3.3.3.2</li>
  </ol>
</div> </li>
  </ol>
</div> </li>
  </ol>
</div> </li>
  </ol>
</div>

...it's not perfect, but better.

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented

356f631: Added explicit class="{{ attributes.class }}" back to item-list.html.twig, for easy-understandability-of-how-to-inject-our-own-classes, even though it'll ofter result in the class attribute being empty. :(

My thinking was that when we needed to add a class at the template level, we'd create a new template (i.e. in one of the template suggestions), and do this explicit class="" + attributes stuff there, not in stock item-list.html.twig.

Singling out an attribute, like class for example, for this kind of override/injection, should be one of the first thing any D8 Twig themer should learn and understand, instead of it being in the core item-list template (where we'll be **ensuring** at least some frequency of empty class attributes).

steveoliver’s picture

steveoliver CreditAttribution: steveoliver commented
Component: Twig templates » Twig templates conversion (front-end branch)
Assigned: steveoliver » Unassigned
Status: Postponed » Fixed
Issue tags: -Twig, -theme system cleanup, -Theme Component Library

Queue cleanup. Don't see any point in keeping this open or tagged. See core issues #1939062: Convert theme_item_list() to Twig.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Link to existing issue for adding 'type' support to list item children.