[D8] Support custom list types in `theme_item_list()` for children/nested items

[klonos]

This is the respective issue for https://www.drupal.org/project/drupal/issues/1785310 (change record: theme_item_list() supports custom list types for child elements), which was actually part of our list in #166, but never actually got implemented/backported to Backdrop.

As it is currently in D7 and in Backdrop, nested list items within lists generated by theme_item_list() inherit their type (<ul>/<ol>) from the type variable specified in the parent items. This means that you cannot easily/properly create lists where the children items may need to be of a different type than the parent list.

So I was working on another issue, and I needed to generate this sort of a list:

<ol>
  <li>parent item 1
    <ul>
      <li>child 1</li>
      <li>child 2</li>
    </ul>
  </li>
  <li>item 2</li>
  <li>item 3</li>
</ol>

Which I expected be rendered as follows:

1. parent item 1
   • child 1
   • child 2
3. item 2
4. item 3

So I have tried the following code (which works in D8/9):

  $items = array();

  $items[] = array(
    'data' => 'parent item 1',
    'children' => array(
      array(
        'data' => 'child 1',
      ),
      array(
        'data' => 'child 2',
      ),
    ),
    'type' => 'ul',
  );
  $items[] = 'item 2';
  $items[] = 'Item 3';

  $return theme('item_list', array('items' => $items, 'type' => 'ol'));

Which rendered as follows (nested items ignored the ul type I've specified for them, and rendered as <ol> instead - inheriting that from their parent):

1. parent item 1
   1. child 1
   2. child 2
3. item 2
4. item 3

The workaround I resorted to was this:

  $items = array();
  $children = array();

  $children[] = array(
    'data' => 'child 1',
  );
  $children[] = array(
    'data' => 'child 2',
  );

  $items[] = array(
    'data' => 'parent item 1' . theme('item_list', array('items' => $children, 'type' => 'ul')),
  );
  $items[] = 'item 2';
  $items[] = 'Item 3';

  $return theme('item_list', array('items' => $items, 'type' => 'ol'));

That works, but it feels hacky. We should support this the same way as in D9, both because it feels like the proper way to do it, but also because it would allow D9 snippets of code (either backports or things provided in how-to's and solution articles) to be easily converted to Backdrop.

[klonos]

PR for review: https://github.com/backdrop/backdrop/pull/4202

The actual code change is very minimal - only minor edits in a couple of lines in theme_item_list() - the rest of the changes are:

• docblock: additions/clarifications (we'll need to edit the @since line before we merge - this will depend on whether we decide that this is a new feature or a bug/shortcoming or task)
• tests: instead of changing the existing test to include a hard-coded type for nested items (which was basically only testing one possible scenario) like it was done in Drupal, I have adapted the test to check every possible combination of ul/ol for the parent/child items. We have better coverage that way I believe.
• CSS: the change was necessary, because during testing I have faced a situation with the default Seven theme where although the rended HTML had the proper <ol> tag for a nested list inside a <ul> outer list, the CSS was overriding it with list-style-type: disc, making it render as a bullet list instead of a numbered one (made me scratch my head for a while before I figured what was going on). I believe that this is due to some duplication of CSS li-related selectors in core/themes/seven/css/style.css and core/themes/seven/css/seven.base.css. If anyone can figure out a way to fix this without any CSS changes, please let me know.

[klonos]

...re whether this is a new feature or a bug, or a task, I hope that these points may help:

• this is an API addition - not a breaking API change
• The title of this issue could have been "theme_item_list(): not possible to specify nested lists which have a different type from their parent list", which would make it a bug.
• This addition brings us in parity with what is possible with the same function in really early versions of Drupal 8.0.x - it was something that we did not backport when we forked.

The change record for D8 has more details:

theme_item_list() now supports a type attribute to be specified for child list items, allowing child lists to have a different type than their parent.

Previously, the parent type was inherited by all child lists, preventing, for example, an unordered list from being contained within an ordered list.

Now, if type is set for a child list, it will be used. If not, the parent type will be inherited.

[klonos]

I've rebased the PR for this. Keen to get it out of the way if anyone can spare some time for a review please.

This is currently also blocking #6644.

[bugfolder]

I tried this out. After applying the patch, I tried this code:

  $vars = array(
    'type' => 'ol',
    'items' => array(
      'item A',
      'item B',
      array(
        'data' => 'item C',
      ),
      array(
        'data' => 'item D',
        'type' => 'ul',
        'children' => array(
          'item E',
          'item F',
          array(
            'data' => 'item G',
          ),
          array(
            'data' => 'item H',
            'type' => 'ol',
            'children' => array(
              'item I',
              array(
                'data' => 'item J'
              ),
            ),
          ),
        ),
      ),
    ),
  );
  $build[] = array(
    '#markup' => theme('item_list', $vars),
  );

which rendered like this: which is what we want. WFM.

Reviewed the code, it looks good. LGTM.

One small suggestion on the code, which I don't feel strongly about, but will make anyhow.

[klonos]

Thanks @bugfolder 🙏🏼 ...I've implemented the suggestion + added a note that if no type is specified, then it is inherited from the parent list, and then also made a similar change for the description of the type for the parent list (no e.g. - singl instead of double quotes etc.).

Setting a tentative milestone. Please have another look at the docblock changes and mark RTBC if you think this is ready.

[bugfolder]

Looks good, RTBC!

[quicksketch]

This looks great but I think we can avoid new CSS selectors in the Seven theme by removing an unnecessary selector. See my feedback at https://github.com/backdrop/backdrop/pull/4202#pullrequestreview-2244556185.